merlin icon indicating copy to clipboard operation
merlin copied to clipboard

Illustrate and fix issue #1610

Open voodoos opened this issue 2 years ago • 2 comments

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:

  1. reconstruct_identifer looks broken on module paths
  2. Locate used Longident.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 path M(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).

voodoos avatar May 22 '23 14:05 voodoos

@let-def I had to modify reconstruct_identifierfor that patch.

The issue was that "papply" components in paths where dropped leading to the impossibility to jump to t in N(F).t. I decided to go with a quick, but kind-of dirty trick: application are considered a single component. A path M.N(F).t would be reconstructed as ["M";"N(F)";"t"].

This does fix the issue but prevent users from jumping to N of F.

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...

voodoos avatar May 23 '23 13:05 voodoos

@let-def I tried to get reconstruct_identifier to work with Papply components in 0d8bdd9 but it's not very satisfying. I guess we should really have a different output type than a flat list ? Have you thought about it already ? I am surprised that this was not handled so far.

In any case, the current hacky solution works on the test suite but it has an unexpected side-effect: When looking fo the type of t in M(N).t reconstruct identifier now correctly returns the list of reconstructed identifiers: ["M(N).t"] instead of an empty list. However parsing that expression fails: Parser_raw.parse_expression mistakenly returns Pexp_construct and Pexp_field nodes and so type-enclosing is wrong. Do you now how I could fix this ?

voodoos avatar May 24 '23 09:05 voodoos