Use Merlin Enclosing command instead of Shape to answer SelectionRange queries

[voodoos]

The enclosing command is the dedicated Merlin command to provide meaningful growing and shrinking of the selection.

[rgrinberg]

Does this still work when the module fails to type check?

Out of curiosity, what are "Shapes" useful for if not for this?

Please update CHANGES.

[voodoos]

The "shape" command of Merlin has nothing to do with the "shapes" that we use to jump to definitions.
It is an old command that is not even use by the Emacs and Vim modes... In fact the vim mode even relies on the type_enclosing command instead of the more specialized one enclosing.

I just tried and it appears to work when the buffer does not type but I did not test it extensively.

[rgrinberg]

I'm still curious why do we have Shape in merlin at all then?

By the way, I would expect expanding selection wouldn't always select an expression or a type. For example, given (bar signifies the cursor, [ and ] demonstrate the selection)

let ..|.. in

would expand to

let [ ... ] in

and then to

[let ... in]

I'm not sure the current shapes command respects that either, but it would be good if a proper solution for expand selection did.

[rgrinberg]

Finally, does expand selection work inside comments?

[ulugbekna]

TODO(ulugbekna): we should batch these merlin queries into one

[ulugbekna]

Finally, does expand selection work inside comments?

No, the only range returned is the whole document range, but, for example, vscode is smart about it and expands selection gradually: word, parenthesized expression, line, then the whole document.

[Trace - 11:25:34 PM] Sending request 'textDocument/selectionRange - (400)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/ulugbekna/code/ocaml-lsp/ocaml-lsp-server/src/semantic_highlighting.ml"
    },
    "positions": [
        {
            "line": 947,
            "character": 63
        }
    ]
}


[Trace - 11:25:34 PM] Received response 'textDocument/selectionRange - (400)' in 1ms.
Result: [
    {
        "range": {
            "end": {
                "character": 72,
                "line": 1027
            },
            "start": {
                "character": 0,
                "line": 0
            }
        }
    }
]

What would you expect from expanding selection in a comment? Do you mean the selections could work in pre-formatted code block within a comment {| |} ?

[voodoos]

I'm not sure the current shapes command respects that either, but it would be good if a proper solution for expand selection did.

Yes, both the shapesand enclosing commands use the same data which is the typedtree to provide the selections so the results should be mostly the same and in both case they only return selections that corresponds to valid typedtree nodes.

It looks like the kind of selection you describe would be more "parsetree" based.

I'm still curious why do we have Shape in merlin at all then?

It looks like a refinement over the outline command that returns only the outline for the current branch of the tree. I guess it was meant to allow easy navigation to parent binders like (|⁰, |¹ are successive positions of the cursor):

|² let x = 3 in„
  |¹ let y = 4 in
     |⁰ x + y

In Merlin's doc:

This command can be used to assist navigation in a source code buffer. It returns a tree of all relevant locations around the cursor. It is similar to outline without telling any information about the entity at a given location.

[rgrinberg]

It looks like the kind of selection you describe would be more "parsetree" based.

That's exactly what I would expect for this command.

This command can be used to assist navigation in a source code buffer. It returns a tree of all relevant locations around the cursor. It is similar to outline without telling any information about the entity at a given location.

At least from this description, it seems like Shape fits better fitted to play the role of "expand selection" than the enclosing type.

[ulugbekna]

Do we plan to merge this? @rgrinberg

Would be great to merge before there're more conflicts. I can fix the conflict if @voodoos is busy

[voodoos]

Rebased.

At least from this description, it seems like Shape fits better fitted to play the role of "expand selection" than the enclosing type.

Right, and it does look like the code on Merlin's side is doing some interesting selection to reduce noise, however the result are still bad. Maybe it is worth to have a look and fix it. But it is quite low on my priority list.

This PR would make the behavior consistent with how Merlin behave in Emacs in Vim, but this might not be "better". I will try to find the type to write some test that highlight differences between the two commands.

[rgrinberg]

I'd like to see some tests before we evaluate the PR.

As I've mentioned earlier, I don't think growing the type enclosing is the correct interpretation for growing the selection. Shapes might not be any better, but we need to agree how it should work first.