Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vim.lsp.buf.code_action should get diagnostics at cursor, not line #21985

Open
zhoudaxia2016 opened this issue Jan 24, 2023 · 7 comments · May be fixed by #25747
Open

vim.lsp.buf.code_action should get diagnostics at cursor, not line #21985

zhoudaxia2016 opened this issue Jan 24, 2023 · 7 comments · May be fixed by #25747
Labels
bug issues reporting wrong behavior complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md has:plan lsp
Milestone

Comments

@zhoudaxia2016
Copy link
Contributor

Describe the bug

Use typescript-language-server and vim.lsp.buf.code_action returns wrong code actions
I thinkvim.lsp.buf.code_action parameter diagnostics should be filtered by col.
The related code is:
https://github.com/neovim/neovim/blob/master/runtime/lua/vim/lsp/buf.lua#L765

Steps to reproduce

  1. add a ts file like that
function a(x/**cursor here**/) {
  console.log(x)
}
export const b = 1
  1. put the cursor at parameter x
  2. execute vim.lsp.buf.code_action
  3. the code action list has item Remove unused declaration for: 'x'

Expected behavior

the code action list has not Remove unused declaration for: 'x'

Neovim version (nvim -v)

NVIM v0.9.0-dev-7ef5e363d-dirty

Vim (not Nvim) behaves the same?

no

Operating system/version

windows wsl2

Terminal name/version

window terminal 1.15

$TERM environment variable

xterm-256color

Installation

build from repo

@zhoudaxia2016 zhoudaxia2016 added the bug issues reporting wrong behavior label Jan 24, 2023
@glepnir glepnir added the lsp label Jan 24, 2023
@glepnir
Copy link
Member

glepnir commented Jan 24, 2023

i am not sure what design of there . but i think you can do it at local . get diagnostic at cursor and pass it to context. I am not test these code . write in github :)

local function get_diagnostic_at_cursor()
  local cur_buf = api.nvim_get_current_buf()
  local line, col = unpack(api.nvim_win_get_cursor(0))
  local entrys = diagnostic.get(cur_buf, { lnum = line - 1 })
  local res = {}
  for _, v in pairs(entrys) do
    if v.col <= col and v.end_col >= col then
      table.insert(res, {
        code = v.code,
        message = v.message,
        range = {
          ['start'] = {
            character = v.col,
            line = v.lnum,
          },
          ['end'] = {
            character = v.end_col,
            line = v.end_lnum,
          },
        },
        severity = v.severity,
        source = v.source or nil,
      })
    end
  end
  return res
end

vim.lsp.buf.code_action({
 context = {
      diagnostics = get_diagnostic_at_cursor()
}})

@zhoudaxia2016
Copy link
Contributor Author

i am not sure what design of there . but i think you can do it at local . get diagnostic at cursor and pass it to context. I am not test these code . write in github :)

local function get_diagnostic_at_cursor()
  local cur_buf = api.nvim_get_current_buf()
  local line, col = unpack(api.nvim_win_get_cursor(0))
  local entrys = diagnostic.get(cur_buf, { lnum = line - 1 })
  local res = {}
  for _, v in pairs(entrys) do
    if v.col <= col and v.end_col >= col then
      table.insert(res, {
        code = v.code,
        message = v.message,
        range = {
          ['start'] = {
            character = v.col,
            line = v.lnum,
          },
          ['end'] = {
            character = v.end_col,
            line = v.end_lnum,
          },
        },
        severity = v.severity,
        source = v.source or nil,
      })
    end
  end
  return res
end

vim.lsp.buf.code_action({
 context = {
      diagnostics = get_diagnostic_at_cursor()
}})

This code work well

@justinmk justinmk added this to the backlog milestone Jan 26, 2023
@justinmk justinmk added good-first-issue complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md has:plan labels Jan 26, 2023
@ryand67

This comment was marked as off-topic.

@dundargoc

This comment was marked as off-topic.

@to-json
Copy link

to-json commented Apr 2, 2023

I messed around and wrote a patch to incorporate this function into vim.buf.lsp and call it by default, but, in a current version, I am unable to reproduce: column based behaviour is present by default. attaching a minimal repro env

dockerfile:

FROM debian:bookworm

WORKDIR '/test'
RUN apt update
RUN apt install -y git ninja-build gettext libtool-bin cmake g++ pkg-config unzip curl npm nodejs gnutls-bin
RUN npm install -g typescript-language-server typescript
RUN git clone https://github.com/neovim/neovim.git
RUN bash -c "pushd neovim; make"
ADD ./nvim-mnml /root/.config/nvim
ADD ./test.ts /test/test.ts

nvim-mnml/init.lua:

vim.lsp.start({
  name = 'tsserver',
  cmd = {'typescript-language-server', '--stdio'},
  root_dir = vim.fs.dirname(),
})

vim.keymap.set('n', '<Leader>a', function()vim.lsp.buf_attach_client(0,1)end)

vim.api.nvim_create_autocmd('LspAttach', {
  callback = function(args)
    vim.keymap.set('n', 'K', vim.lsp.buf.hover, { buffer = args.buf })
    vim.keymap.set('n', '<Leader>a', vim.lsp.buf.code_action, { buffer = args.buf })
  end,
})

test.ts:

function a(x/**cursor here**/) {
  console.log(x)
}
export const b = 1

I'm less sure -why- it currently works correctly, but, it does.
docker build --tag nvim-test . ;docker run -t -i nvim-test bash -c 'cd neovim/runtime/lua; /test/neovim/build/bin/nvim /test/test.ts' to drop into a test env, Leader-a to attach, and then again to run code actions. Leader should naturally be backslash.

edit: i can naturally push the patch if that's useful, lmao, but, seems inadvisable

second edit: i guess it could be a wsl thing somehow? that seems bonkers tho, and not worth fixing nvim side

@to-json
Copy link

to-json commented Apr 2, 2023

followup: the current code_action impl uses the deprecated get_line_diagnostics and wants a rewrite for that reason anyway, so i might just do that, buuut that involves essentially rewriting get_line_diagnostics in the buf namespace in terms of diagnostic.get. is that actually the desired outcome of deprecating get_line_diagnostics? the other thought i had, since get currently takes a line, was to amend it to also take a col or cursor. given that I don't actually understand the choice to deprecate the helper api, gonna hold on that a moment until this issue gets feedback

@faergeek
Copy link
Contributor

Seems like it should get diagnostics not only exactly at cursor position, but also diagnostics which "cover" cursor position, if diagnostic is multiline

gpanders added a commit to gpanders/neovim that referenced this issue Oct 22, 2023
Instead of using diagnostics from the entire cursor line to find
available code actions, only consider diagnostics whose range overlaps
the current cursor position.

This will also code actions for diagnostics that span multiple lines.

Fixes: neovim#21985
gpanders added a commit to gpanders/neovim that referenced this issue Oct 22, 2023
Instead of using diagnostics from the entire cursor line to find
available code actions, only consider diagnostics whose range overlaps
the current cursor position.

This will also code actions for diagnostics that span multiple lines.

Fixes: neovim#21985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md has:plan lsp
Projects
None yet
7 participants