Merlins Jump in Ocaml-LSP

[PizieDust]

closes #1360

Jump to Target

Merlin's Jump command allows precise code navigation in a source code buffer. This functionality is particularly useful for developers who wish to streamline their navigation within large codebases by jumping directly to relevant code segments. However, the standard LSP protocol only allows for generalized code navigation (goto definition, goto declaration, goto implementation, goto type-definition), which is not very useful when it comes to precise movements.

The Jump command in Merlin allows us to jump to a set of targets:

• fun: Jump to a function
• let: Jump to a let definition
• module: Jump to a module
• module-type: Jump to a module type definition
• match: Jump to a match construct.
• match-next-case: Jump to the next case in the match body.
• match-prev-case: Jump to the previous case in the match body.

While this Jump functionality can be implemented in ocaml-lsp using a custom request, see Closed PR - Jump | Custom Request, we sought to find different solutions as using a custom request requires individual client implementations for all the clients we may want to support.

This PR implements Merlin's jump command in ocaml-lsp using a new set of CodeActions.

Implementation

The implementation of the "Jump to Target" feature uses a CodeAction with a showDocumentRequest to move the cursor to the required position.

1. Document Detection

When a source file is opened, the server checks if the document is an OCaml file supported by Merlin. If the document is not compatible with Merlin, the "Jump to Target" code action is not provided.

2. Client Capability Check

Before generating code actions, the server verifies whether the client supports the ShowDocument client capability. If this capability is not supported, the "Jump to Target" feature is disabled.

3. Generating Code Actions with Context

For each target (fun, let, module, etc.), the server attempts to locate the corresponding position in the document using Merlin's Jump command:

• The server asynchronously sends jump requests for each target, allowing for non-blocking execution.
• The results are collected, and for each successful jump (i.e., where Merlin finds a valid target position), a CodeAction is created.
• Each CodeAction includes a title, such as "Jump to function", and a command (ocamllsp/merlin-jump-to-target) that instructs the client to jump to the specified location in the document.

4. Command Execution

When a user selects a code action, the associated command is executed. The server sends a ShowDocument request to the client, which asks the client to focus on the URI and range where the target was found.

5. Error Handling

The implementation includes error handling to ensure that if a jump request fails (e.g., Merlin does not find the target or the position is invalid), the code action may be omitted.

Advantages of the Code Action Approach

• Client-Agnostic: By implementing this functionality as a CodeAction, the solution is broadly compatible with different clients. It avoids the need for custom client implementations that would have been required with a custom LSP request.
• Asynchronous Execution: The use of Fibers allows for non-blocking execution of jump requests, ensuring that the server remains responsive even when processing multiple targets in parallel.
• Precision Navigation: Users can quickly navigate to specific constructs within their OCaml code, improving productivity and making it easier to work within large and complex codebases.
• Integration with Existing LSP Features: This implementation uses existing LSP features, providing a familiar experience for users while extending the capabilities of the ocaml-lsp-server.

Demo (VSCode)

merlin.jump.test.mp4

Resources

• LSP Protocol - ShowDocument
• LSP Protocol - ExecuteCommand

cc @voodoos @xvw @pitag-ha

[coveralls]

Pull Request Test Coverage Report for Build 4501

Details

• 27 of 43 (62.79%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.07%) to 21.851%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
ocaml-lsp-server/src/ocaml_lsp_server.ml	0	2	0.0%
ocaml-lsp-server/src/code_actions/action_jump.ml	22	36	61.11%

Totals
Change from base Build 4495:	0.07%
Covered Lines:	5558
Relevant Lines:	25436

---

💛 - Coveralls

[voodoos]

Thanks Pizie, I think this is a good compromise to actually provide jump functionnality without having to provide client-side support.

For reference, here is the trace of executing such a code action:

1. First the client request the list of code actions and the server answers:

[Trace - 11:47:51] Sending request 'textDocument/codeAction - (71)'.
Params: {
    "textDocument": {
        "uri": "file:///.../prefix_parser.ml"
    },
    "range": {
        "start": {
            "line": 10,
            "character": 42
        },
        "end": {
            "line": 10,
            "character": 42
        }
    },
    "context": {
        "diagnostics": [],
        "triggerKind": 2
    }
}


[Trace - 11:47:51] Received response 'textDocument/codeAction - (71)' in 7ms.
Result: [
    ...,
    {
        "command": {
            "arguments": [
                "file:///.../prefix_parser.ml",
                {
                    "end": {
                        "character": 2,
                        "line": 9
                    },
                    "start": {
                        "character": 2,
                        "line": 9
                    }
                }
            ],
            "command": "ocamllsp/merlin-jump-to-target",
            "title": "Jump to let"
        },
        "kind": "merlin-jump",
        "title": "Jump to let"
    }
]

2. Then if the user choose this code action the client ask the server to execute the appropriate command which will make the server send a showDocument request.

[Trace - 11:47:56] Sending request 'workspace/executeCommand - (73)'.
Params: {
    "command": "ocamllsp/merlin-jump-to-target",
    "arguments": [
        "file:///.../prefix_parser.ml",
        {
            "end": {
                "character": 2,
                "line": 9
            },
            "start": {
                "character": 2,
                "line": 9
            }
        }
    ]
}


[Trace - 11:47:56] Received request 'window/showDocument - (6)'.
Params: {
    "selection": {
        "end": {
            "character": 2,
            "line": 9
        },
        "start": {
            "character": 2,
            "line": 9
        }
    },
    "takeFocus": true,
    "uri": "file:///.../prefix_parser.ml"
}

It's a nice trick, but it is a bit sad to have the additional round-trip when all the information is already stored in the codeAction response. Do you think it would be possible to have custom client support that jump directly and fallback to the showDocument trick for clients without custom support ?

[rgrinberg]

It's a nice trick, but it is a bit sad to have the additional round-trip when all the information is already stored in the codeAction response. Do you think it would be possible to have custom client support that jump directly and fallback to the showDocument trick for clients without custom support

Is this slowdown observable in practice? LSP already contains a ton of inefficiencies (some essential to the protocol, others are our implementation of it). I would expect such a roundtrip to be unimportant given all the LSP traffic.

[xvw]

Just two small nitpickings and one question from my review perspective. Thanks @PizieDust !

[voodoos]

Looks good to me :-)

[xvw]

👍🏾

[awilliambauer]

I think this approach precludes users being able to trigger these movements via keyboard shortcut because they'll always have to code through the code actions menu. Since all the items begin with the same letters, the user can't easily use the keyboard to jump to one of them in the code actions menu either.

[voodoos]

I think this approach precludes users being able to trigger these movements via keyboard shortcut because they'll always have to code through the code actions menu. Since all the items begin with the same letters, the user can't easily use the keyboard to jump to one of them in the code actions menu either.

So there is a way to add shortcuts to code actions (for example alt-d is destruct in the vscode-ocaml-platform plugin), but since all the jump commands have the same "kind" it's indeed problematic here:

A solution to that problem would be to use different kinds for the different jumps "merlin-jump-let so that users could make their custom keybindings. wdyt @awilliambauer ?

For reference, here is the keybindings.json file I used to configure the shortcut on the code action:

// Place your key bindings in this file to override the defaults
[
    {
        "command": "editor.action.codeAction",
        "key": "Alt+J",
        "args": {
        "kind": "merlin-jump"
        },
        "when": "editorLangId == ocaml"
    },
]

[awilliambauer]

A solution to that problem would be to use different kinds for the different jumps "merlin-jump-let so that users could make their custom keybindings. wdyt @awilliambauer ?

This would be a good solution!

[voodoos]

A solution to that problem would be to use different kinds for the different jumps "merlin-jump-let so that users could make their custom keybindings. wdyt @awilliambauer ?

This would be a good solution!

cc @PizieDust 😉

[awilliambauer]

I've discussed this with other JS editors folks and have a couple other concerns:

• These code actions add a lot of clutter to the code actions menu. Would there be a way to suppress them from being displayed in the menu, but still make them available via keyboard shortcut?
• There are various ways editors might want to incorporate these jump commands into a large commend (e.g., a macro that jumps to a let and adds a specific annotation). We'd also want the user to be able to navigate the history of these jumps (i.e., jump backwards and forwards). Having these as code actions instead of a dedicated custom query will probably make supporting both of these things a lot harder.

[voodoos]

I've discussed this with other JS editors folks and have a couple other concerns:

* These code actions add a lot of clutter to the code actions menu. Would there be a way to suppress them from being displayed in the menu, but still make them available via keyboard shortcut?

* There are various ways editors might want to incorporate these jump commands into a large commend (e.g., a macro that jumps to a let and adds a specific annotation). We'd also want the user to be able to navigate the history of these jumps (i.e., jump backwards and forwards). Having these as code actions instead of a dedicated custom query will probably make supporting both of these things a lot harder.

Right, I guess that there are not other way around than a custom query if we want that kind of flexibility. cc @rgrinberg @PizieDust