yegappan / lsp

Language Server Protocol (LSP) plugin for Vim9
MIT License
485 stars 57 forks source link

Only jump to the first implementation, ignoring other implementations #171

Closed ycycyyc closed 1 year ago

ycycyyc commented 1 year ago

When I am writing a program in golang, GotoImplementation method will only jumps to the first implementation, and ignore other implementations.

https://github.com/yegappan/lsp/blob/main/autoload/lsp/lspserver.vim#L905 If reply->result is a list, why only the first value is taken?

It maybe better to create a location list with the location of the impls.

andlrc commented 1 year ago

I think that it makes sense to hydrate the location list in such cases where multiple matches are found. Keep in mind that the functions is also used to "peek" at a implementation.

Currently you can set tagfunc to lsp#lsp#TagFunc which will support multiple matches with :h :tj, :h :tn et al.

lsp#lsp#TagFunc however only supports textDocument/definition and not textDocument/implementation, we might consider adding support for changing this as a user:

g:LspOptionsSet({ tagMethod: 'textDocument/implementation' })

Or possible add support for multiple methods:

g:LspOptionsSet({ tagMethods: ['textDocument/implementation', 'textDocument/definition'] })

But this means that is one wants to always jump to something, then use tagfunc otherwise the commands directly.


I can generate multiple definitions with the following typescript code:

$ cat b.ts
export function B(val: number): void;
export function B(val: string): void;
export function B(val: string | number) {
        console.log(val);
        return void 0;
}

$ cat a.ts
import { B } from './b'; // jumps to line 1 (there are _actually_ three matches)  
B(1);                    // jumps to line 1 (currect)
B('1');                  // jumps to line 2 (currect) 
ycycyyc commented 1 year ago

Thanks for the quick reply! lsp#lsp#TagFunc however only supports textDocument/definition. So I rewrote the GotoSymbolLoc function, and temporarily added fzf to display multiple entries. ShowReferences can also show multiple entries. The following code and pictures below:

package main

type A/*goto implementation*/ interface {
        Hello()
}

type B struct{}

func (b *B) Hello() {}

type C struct{}

func (c *C) Hello() {}

func main() {
}

image

It is expected that users can customize the handler in the future.

yegappan commented 1 year ago

Yes. It makes sense to create a location list when there are multiple locations/matches for a definition, declaration, etc. The method used by 'tagfunc' should also be a configurable option.

Can you show the diff that you used to integrate FZF for displaying the locations?

andlrc commented 1 year ago

@ycycyyc can you take a look at https://github.com/yegappan/lsp/pull/174 and see if it works as you would expect.

ycycyyc commented 1 year ago

@ycycyyc can you take a look at #174 and see if it works as you would expect.

Nice work! thanks @andlrc

ycycyyc commented 1 year ago

Yes. It makes sense to create a location list when there are multiple locations/matches for a definition, declaration, etc. The method used by 'tagfunc' should also be a configurable option.

Can you show the diff that you used to integrate FZF for displaying the locations?

@yegappan Actually I refer to this repository https://github.com/ojroques/nvim-lspfuzzy to add fzf support. Diff is here: https://github.com/yegappan/lsp/compare/main...ycycyyc:lsp:main. This commit is only temporary and for my own use. There are still many bugs that have not been fixed. It would be even better if this issue https://github.com/yegappan/lsp/issues/148 can be implemented.