Improve context detection for infix operators #1756
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xvw
reviewed
May 2, 2024
Comment on lines
+70
to
+71
| (* FIXME: this is britle, but lids don't have precise enough location | ||
| information to handle these cases correctly. *) |
There was a problem hiding this comment.
I think it is brittle. But in fact, is that really brittle?
There was a problem hiding this comment.
Well, the whole thing is a bit dodgy. Merlin reconstructs a string, that it then parses again, to try to guess on what component the cursor here since longident don't have precise enough location information. Mixed with weird things like ident that require parenthesis or others that require parenthesis plus spaces, it makes the whole process very hacky.
Hopefully having better location information in longidents will allow us to replace all of this soon :-)
There was a problem hiding this comment.
Make sense. Thanks for the explanation!
xvw
approved these changes
May 2, 2024
voodoos
force-pushed
the
improve-context-detection
branch
from
3024444
to
b551ab2
Compare
It's always correct to have a space after the parenthesis and it is sometimes necessary, for infix operators that start with `*` for example.
voodoos
force-pushed
the
improve-context-detection
branch
from
b551ab2
to
9f8cbf4
Compare
voodoos
added a commit
to voodoos/merlin
that referenced
this pull request
May 13, 2024
Improve context detection for infix operators
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
May 17, 2024
CHANGES:
Fri May 17 19:59:42 CET 2024
+ merlin binary
- Support for OCaml 5.2 (ocaml/merlin#1757)
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
- Addition of a `merlin-lib.commands` library which disassociates the
execution of commands from the `new_protocol`, from the binary, allowing
it to be invoked from other projects (ocaml/merlin#1758)
- New occurrences backend: Don't index occurrences when `merlin.hide`
attribute is present. (ocaml/merlin#1768)
- Use the new `uid_to_decl` table in 5.2's cmt files to get documentation.
(ocaml/merlin#1773)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
May 31, 2024
CHANGES:
May May 31 14:02:42 CET 2024
+ merlin binary
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
May 31, 2024
CHANGES:
Fri May 31 14:02:42 CEST 2024
+ merlin binary
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
May 31, 2024
CHANGES:
Fri May 31 14:02:42 CEST 2024
+ merlin binary
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
May 31, 2024
CHANGES:
Fri May 31 14:02:42 CEST 2024
+ merlin binary
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
avsm
pushed a commit
to avsm/opam-repository
that referenced
this pull request
Sep 5, 2024
CHANGES:
Fri May 31 14:02:42 CEST 2024
+ merlin binary
- destruct: Removal of residual patterns (ocaml/merlin#1737, fixes ocaml/merlin#1560)
- Do not erase fields' names when destructing punned record fields (ocaml/merlin#1734,
fixes ocaml/merlin#1661)
- Ignore SIGPIPE in the Merlin server process (ocaml/merlin#1746)
- Fix lexing of quoted strings in comments (ocaml/merlin#1754, fixes ocaml/merlin#1753)
- Improve cursor position detection in longidents (ocaml/merlin#1756)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context detection for longidents relies on some britle "where is the cursor in the lid" logic.
This PR add a special case to improve the handling of operators that requires parenthesis.
More precise locations in lid is required to actually get a clean and correct implementation, but at least this PR improves the situation a bit.