-
Notifications
You must be signed in to change notification settings - Fork 233
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
Illustrate and fix issue #1610 #1611
base: main
Are you sure you want to change the base?
Conversation
@let-def I had to modify The issue was that "papply" components in paths where dropped leading to the impossibility to jump to This does fix the issue but prevent users from jumping to I though this wasn't supported before anyway, but I think I am wrong about that, that's why I cancelled my request for review. I guess there is no working around a cleaner implementation without breaking existing behavior... |
This fixes locate when a prefix is given, but reconstruct_identifier is still not giving the correct answer. This also fix another issue with infix operators.
about locating infix operators
This treat an application as a single components so it is not a satisfing long-term solution. A better approach would be to change the return type of [reconstruct_identifier] to account for module application. This fixes ocaml#1610
91507bd
to
0d8bdd9
Compare
@let-def I tried to get In any case, the current hacky solution works on the test suite but it has an unexpected side-effect: |
This PR add tests illustrating issue #1610: locate not jumping correctly with path such as
M(C).t
.There are two things leading to this:
reconstruct_identifer
looks broken on module pathsLongident.parse
to parse which is also broken on module path (and infix operators).This PR improve 2 by using
Parser.parse_longident
. This make locate work when the pathM(C).t
is manually inputted by the user. This also fixes another issue related to infix operators.I will keep that PR as a draft until 1 is also fixes (and thus the issue).