Conversation
e31d8b0 to
d3a9d49
Compare
|
There is an issue with let () =
f ();
( f ();
f () )and let () =
f ();
f ();
f ()As a consequence, we might include let () =
[f ();
( f ();]
f () )I'll investigate, but I'm not sure how accessible those locations are... I would say this PR is already mergeable as is. I'll include the problem as a FIXME in the test. |
dcb4244 to
c29d2a5
Compare
f08b060 to
4105532
Compare
4105532 to
edc9668
Compare
edc9668 to
b1aaaf1
Compare
b1aaaf1 to
d2d15e3
Compare
|
I've reworked the history to make it easier to review, since the last commits introduced a few "special cases". The first commit is the core of the rewrite of
The next commits fix those FIXME one by one, and each of them is fairly small. |
Allow incomplete node to be selected. This is the base implementation, there are several corner cases that are left as FIXME in the test and are going to be fixed in the next commits. The idea is still "going up nodes from the cursor". However, this time we can also go "down right" to select a node in several step. For instance, we don't go from: ```ocaml let x = [1] in let y = 1 in x + y ``` to the parent node: ```ocaml [let x = 1 in let y = 1 in x + y] ``` Instead, we include "intermediate steps for the parent node" by filling it from left to right: ```ocaml [let x = 1 in] let y = 1 in x + y ``` then ```ocaml [let x = 1 in let y = 1 in] x + y ``` and finally ```ocaml [let x = 1 in let y = 1 in x + y] ``` Co-authored-by: PizieDust <[email protected]>
Instead of starting from scratch
Require adding some attributes since the loc information was gone.
They are non-splittable because they are nested in a `begin ... end` or parenthesis.
276a47d to
7cca587
Compare
|
CI is green, this is ready for review! |
|
For the last 4 commits: I made an AI review, added the relevant findings as tests, and fixed them. |
|
Thanks @panglesd |
2 participants
Made with @PizieDust.
Drastically improve the enclosing request.
Going down-right
It used to follow exactly the
Mbrowse.nodehierarchy. But we don't always want to select "whole nodes", as for instance in:we don't want to go from
to:
although this is the parent expression. It would be downright bad!
Instead, the new implementation goes "down-right" on expression nodes when it comes from the left:
then
then
...
And I say down-right, because the (virtual) AST looks like that:
Better locations
This PR improves locations by adding a merlin loc to sequences (
expr1 ; expr2). It is also careful with which kind of locations to consider.Care for constraints
The following expression
((((x : int) : int) : int) : int)is a single expression node... the constraints (and their location) being in theexp_extrafield of the expressionThis PR makes different enclosing for each layer.
Conclusion
I can clean the history, but it should be ok to review it all at once (not commit by commit) since we went in several wrong directions, but at then end it is not too big.
🎸