kkharji / archive-323

The neovim language-server-client UI
MIT License
410 stars 43 forks source link

Error when preview definition #38

Open tienla0409 opened 2 years ago

tienla0409 commented 2 years ago

Description

LspSaga preview_definition() not correct working

Expected Behavior

Actual Behavior

Neovim version v0.6.0-dev

Branch use Main

kkharji commented 2 years ago

which branch, if main what nvim version are you on?

typoon commented 2 years ago

I am having the same issue. I am using the main branch.

:version output:

:version
NVIM v0.6.0-dev+594-g36538417f
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/gcc-11 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -DNVIM_TS_HAS_SET_MATCH_LIMIT -O2 -g -Og -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wm
issing-prototypes -Wimplicit-fallthrough -Wvla -fstack-protector-strong -fno-common -fdiagnostics-color=always -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -D
MIN_LOG_LEVEL=3 -I/home/runner/work/neovim/neovim/build/config -I/home/runner/work/neovim/neovim/src -I/home/runner/work/neovim/neovim/.deps/usr/include -I/usr/include -I/home/runner/work/neovim/neovim/build/src/n
vim/auto -I/home/runner/work/neovim/neovim/build/include
Compiled by runner@fv-az65-601

Features: +acl +iconv +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/share/nvim"

Screenshot of error:

image

DavidePatria commented 2 years ago

I would try using the lsp builtin function :lua vim.lsp.buf.definition() to see if this one returns a definition. In general the function :lua require('lspsaga.provider').preview_definition(), which I have substituted to the regular gd mapping from lspconfig, returns an error when a definition is right below the cursor, and only for some words. The error:

stack traceback:                                                                                                                                                                                                                                                                                                     
        ...onfig/nvim/plugins/lspsaga.nvim/lua/lspsaga/provider.lua:501: in function 'preview_definition'
        [string ":lua"]:1: in main chunk

This happens when searching a definition for keywords such as class or def (using pylsp), cases were lspconfig would simply do nothing if not going to the beginning of the word (remember that lspconfig go to definition function moves to the definition). When doing the same (requesting a definition for a non-keyword function) opens the "useless" hover like shown below.

the "useless" hover from saga for non-keywords: immagine after calling lua require('lspsaga.provider').preview_definition() immagine which is the current portion of code.

Maybe lspsaga should print a message when lspconfig wouldn't move at all, something like "you dumb are right on the definition you are asking".

@typoon does the error always happen or the behaviour is consistent with what I have just described?

typoon commented 2 years ago

So, the case where I am having this issue is in a .js file using tsserver. If I stop the cursor over the forEach function and run :lua vim.lsp.buf.definition() I get redirected to the file lib.es5.d.ts which is the file that contains the definition of forEach.

image

The code I am using is the one below:

    ret = {};
    // Add some stuff to ret here
    Object.keys(ret).forEach(domain => {
        // Do some stuff using the keys from `ret`
    });

Now, if I try to use Lspsaga preview_definition I get:

image

Here is the cursor position for both cases (on top of the letter f in forEach):

image

There are cases in which the Lspsaga preview_definition works. For example in some ruby code I tested it worked fine. I can't say though if this issue happens only with JS code, or other languages as well.

wesselvdv commented 2 years ago

I am having exactly the same issue with both tsserver, and angularls. The :lua vim.lsp.buf.definition() works, but the Lspsaga preview_definition does not.

wesselvdv commented 2 years ago

I added a print(vim.inspect(result)) just before https://github.com/tami5/lspsaga.nvim/blob/859b56479506b837e9990c01659c24bf505fc216/lua/lspsaga/provider.lua#L500

It's showing me a table that starts with index 2, instead of 1:

{
  [2] = {
    result = { {
        originSelectionRange = {
          end = {
            character = 15,
            line = 53
          },
          start = {
            character = 3,
            line = 53
          }
        },
        targetRange = {
          end = {
            character = 18,
            line = 62
          },
          start = {
            character = 6,
            line = 62
          }
        },
        targetSelectionRange = {
          end = {
            character = 18,
            line = 62
          },
          start = {
            character = 6,
            line = 62
          }
        },
        targetUri = "file:///<snip>"
      } }
  }
}

Hence the error in my situation. I don't know anything about Lua specifically, but I reckon that's a problem.

wesselvdv commented 2 years ago

I think I know what's going on. I found this issue in the nvim repo, apparently vim.lsp.buf.formatting_sync uses the clientId as the key in the results returned. So it's not correct to assume that the result is always under key 1, so this bug only occurs when the client in question is not the one with id: 1 as can be found when doing :LspInfo.

So to fix this, we either need to know about the clientId or use the suggestion in that issue which is to use next() to fetch the next result in the table.

wesselvdv commented 2 years ago

I wrote my own monkey patch using next(), and it's surfaced an interesting situation for me. I basically have two clients who are simultaneously attached to the buffer I am working in. vim.lsp.buf.formatting_sync uses both clients to request the definition preview, but it entirely depends on the order of the clients (seems to be chronologically based on their id), which result is used.

In my case, it's both tsserver and angularls, based on the order in which the clients were started either the result from tsserver is used, or the result from angularls. So it raises the question, how to deal with the following situations:

  1. All of the n attached clients return a valid range?
  2. One of the n attached clients has a valid range?

I'd say that for case 2 it's easy, just grab the result from the client that's actually worth showing. But for the other I am not sure, I think the cleanest would be just to show all the results that are given?

kkharji commented 2 years ago

I'd say that for case 2 it's easy, just grab the result from the client that's actually worth showing. But for the other I am not sure, I think the cleanest would be just to show all the results that are given?

Interesting issue. I'd vote for having their contents merged and split with section titled with the lsp server e.g.

From Angularls: ────────────────────────────

and deciding what comes first based on what is most matching what the user expect i.g. valid range

ashton commented 2 years ago

I'm having the same issue with Rescript language server, doing the same print that @wesselvdv did, I get this:

{ {
    result = {
      range = {
        end = {
          character = 8,
          line = 0
        },
        start = {
          character = 4,
          line = 0
        }
      },
      uri = "file:///Users/matheus.ashton/dev/pessoal/pet-manager/./src/App/Router.res"
    }
  } }

The problem is that result is not a table of tables, is a single table itself

RAprogramm commented 2 years ago

LspSaga preview_definition()

try without brackets. Lspsaga preview_definition in C mode vim/nvim