Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

for_completion: handle end of token situation #1998

Merged
merged 1 commit into from
Jun 14, 2018
Merged

for_completion: handle end of token situation #1998

merged 1 commit into from
Jun 14, 2018

Conversation

let-def
Copy link
Contributor

@let-def let-def commented Jun 10, 2018

When completing, ocamlmerlin-reason inserts an identifier if there is not one yet under the cursor.

The rationale is that completion will introduce an identifier in the buffer. So when starting completion on whitespace, an empty identifier is introduced.
This let the parser produce an AST as if the completion was already done. Then merlin can use the type information around this identifier to guide completion.

Unfortunately, the current code doesn't handle the case where the cursor is right at the end of an existing identifier: it will introduce a second one, confusing the parser.

@jordwalke
Copy link
Member

Thanks for the fast fix, @let-def. I'll test it out.

@jordwalke
Copy link
Member

This is feeling pretty great so far! I'll merge after another day of testing, then cut a new release of all the tools.

@chenglou
Copy link
Member

No release yet please; see #2003

@jordwalke
Copy link
Member

Results of more testing: This is a huge improvement in local merlin editing. I think the latest release regressed the quality of merlin, even though the quality of editing increased. This seems to fix it. No new bugs in completion as far as I've been able to tell.

@jordwalke jordwalke merged commit 57ec3b4 into reasonml:master Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants