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

(re-)allow hovers from language servers of other buffers #22323

Open
jmbuhr opened this issue Feb 19, 2023 · 7 comments · May be fixed by #22598
Open

(re-)allow hovers from language servers of other buffers #22323

jmbuhr opened this issue Feb 19, 2023 · 7 comments · May be fixed by #22598
Labels
bug-regression wrong behavior that was introduced in a previous commit (please bisect) has:bisected issue has been tracked to a specific commit lsp
Milestone

Comments

@jmbuhr
Copy link
Contributor

jmbuhr commented Feb 19, 2023

Describe the bug

I wrote a plugin to enable language features for code embedded in other documents (e.g. markdown code blocks) by maintaining hidden buffers with only the language content and relaying language server requests to those hidden buffers. This enables things like autocompletion, go to definition and hover documentation.

But hover documentation will be broken by cfdf5e6 because it explicitly disables hovers that come from buffers other than the current one.

Steps to reproduce

See quarto-dev/quarto-nvim#32

Expected behavior

Hovers from other buffers can still be shown. Some form of opt-in or opt-out via a flag is necessary to serve both usecases.

Neovim version (nvim -v)

v0.9.0-dev-974+g631775c05d

Vim (not Nvim) behaves the same?

no

Operating system/version

linux

Terminal name/version

kitty

$TERM environment variable

xterm-256color

Installation

apt

@jmbuhr jmbuhr added the bug issues reporting wrong behavior label Feb 19, 2023
@zeertzjq zeertzjq added bug-regression wrong behavior that was introduced in a previous commit (please bisect) lsp has:bisected issue has been tracked to a specific commit and removed bug issues reporting wrong behavior labels Feb 19, 2023
@glepnir
Copy link
Member

glepnir commented Feb 19, 2023

use api.nvim_buf_call can allow you run the function in a target buffer.

@jmbuhr
Copy link
Contributor Author

jmbuhr commented Feb 19, 2023

This does allow me to open up the hover, but the hover won't close again (due to being executed in another buffer) and it breaks the other lsp requests I am handling like go to defintion.

The cleanest solution would be a different solution to cfdf5e6 , as this makes hovers handle differently from other lsp requests and introduces arbitrary constraints. I will try to come up with something as well. In its current form I think this is a regression.

@glepnir
Copy link
Member

glepnir commented Feb 19, 2023

also why you don't try rewrite the default handler in your local ?

vim.lsp.handler['textDocument/hover'] = function(_, result)
  --your logic.
end

or send the request by yourself by using lsp.buf_request lsp.buf_request_all

@jmbuhr
Copy link
Contributor Author

jmbuhr commented Feb 19, 2023

Thanks for the suggestion! But won't this break or not work for users that configured their handlers e.g. to use a certain type of border for hover windows?

I think the default handler shouldn't make an assumption about the buffer, since all the other handlers don't do so either (this is handled by the request and response content). The commit I linked that introduces this assumption also mentions that it is a workaround for which we would eventually need a better solution. So I would prefer to not release it in in its current form to not break uses of the api. Otherwise I'll end up with a workaround around a workaround in my plugin.

@glepnir
Copy link
Member

glepnir commented Feb 19, 2023

like i said you can use lsp.buf_request or lsp.buf_request_all to do it in your plugin. it not break any handlers.

 vim.lsp.buf_request(buffernumber, 'textDocument/hover', params, function(_, result, ctx)
         -- your logic
 end)

examplay what i do in lspsaga https://github.com/glepnir/lspsaga.nvim/blob/main/lua/lspsaga/hover.lua

@jmbuhr
Copy link
Contributor Author

jmbuhr commented Feb 19, 2023

Thanks you for your helpful comments! I am indeed already using that and have now added my own handler to handle hovers (just a copy and past of the nvim one without the buffer check) here: jmbuhr/otter.nvim@f29a9f3. This works, but can be improved.

By handling the hover in the plugin I am loosing user configuration done with vim.lsp.with, e.g. a user may have something like this in their config:

    vim.lsp.handlers["textDocument/hover"] = vim.lsp.with(vim.lsp.handlers.hover,
      { border = ....... })

I tried various debugging functions to get the config out of the overwritten handler function, but was unable to to so.
Do you know if this is possible? Thanks for all your help!

@justinmk justinmk added this to the 0.9 milestone Mar 9, 2023
justinmk added a commit to justinmk/neovim that referenced this issue Mar 10, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce `vim.on_before()`.

Example:

    vim.on_before(vim.lsp.handlers, 'textDocument/definition', function()
      vim.notify('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
@justinmk justinmk linked a pull request Mar 10, 2023 that will close this issue
justinmk added a commit to justinmk/neovim that referenced this issue Mar 10, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce `vim.on_before()`.

Example:

    vim.on_before(vim.lsp.handlers, 'textDocument/definition', function()
      vim.notify('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
@justinmk
Copy link
Member

justinmk commented Mar 10, 2023

I tried various debugging functions to get the config out of the overwritten handler function, but was unable to to so.

With #22598 you could sniff the config via:

vim.on_fun(vim.lsp.handlers, 'textDocument/hover', function(...)
  return function(err, result, ctx, config)
    -- `config` passed here is the user-specified config.
  end, ...
end)

this is kinda hacky though.

justinmk added a commit to justinmk/neovim that referenced this issue Mar 11, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 11, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 11, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 11, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 12, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 12, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 13, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 15, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 15, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
justinmk added a commit to justinmk/neovim that referenced this issue Mar 20, 2023
Problem:
No easy way to hook into LSP request/response handlers. (No mention of
"before" or "after" in `:help lsp`...)

Solution:
Introduce vim.on_fun(), a generic way to "hook into" any function
before/after it is called.

Example:

    vim.on_fun(vim.lsp.handlers, 'textDocument/definition', function()
      vim.print('before, yay')
    end)

Fixes neovim#20568
Fixes neovim#22075
Fixes neovim#22323
Related: neovim#13977
@bfredl bfredl modified the milestones: 0.9, 0.10 Apr 7, 2023
@justinmk justinmk modified the milestones: 0.10, 0.11 Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-regression wrong behavior that was introduced in a previous commit (please bisect) has:bisected issue has been tracked to a specific commit lsp
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants