elixir-lsp / elixir-ls

A frontend-independent IDE "smartness" server for Elixir. Implements the "Language Server Protocol" standard and provides debugger support via the "Debug Adapter Protocol"
https://elixir-lsp.github.io/elixir-ls/
Apache License 2.0
1.5k stars 196 forks source link

Go to definition should jump to the start of the identifier(e.g. myFunc) instead of the keyword (e.g. def) #1088

Closed shinohara-rin closed 4 months ago

shinohara-rin commented 6 months ago

Current behaviour

Use Go to Definition on a function will jump to the start of def keyword of the function, instead of the function identifier. For example when jumping to the definition to foo, it will land the cursor at the start of def like this:

    def foo() do
    ^

current behavior

This is same for modules, macros and etc.

The problem

While the current behaviour is okay to use, it seems to me that it would be more appropriate to jump to the actual identifier of the definition instead of the leading keyword, like most language server implementations did.

For example this is how JavaScript language server handles Go to Definition: javascript

And Python too: python

Expected behaviour

Instead of landing on def, it should land on the start of foo like this:

    def foo() do
        ^
lukaszsamson commented 6 months ago

Locating symbols is actually pretty complex. This code is doing the job https://github.com/elixir-lsp/elixir-ls/blob/master/apps/language_server/lib/language_server/location.ex for functions and types. There are 2 mechanisms - 1 relying on doc chunks for compiled modules and other on metadata extracted from AST for current buffer code. While AST is easier to modify as we have control over what we extract, docs are quite limited. Docs actually store the position of @(type|module)?doc attribute, not the def/module/type. A possible approach would be to either extract the correct positions via regex or use the AST in both cases and only fall back to docs if AST extraction fails to find the position. Also keep in mind that the standard asks to return ranges https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_definition https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#location. We need that to fully address https://github.com/elixir-lsp/elixir-ls/issues/1042

lukaszsamson commented 4 months ago

Closing as this is already tracked in #1042