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

expose lsp handler version of telescope lsp builtins #3196

Open
jmbuhr opened this issue Jul 5, 2024 · 2 comments
Open

expose lsp handler version of telescope lsp builtins #3196

jmbuhr opened this issue Jul 5, 2024 · 2 comments
Labels
enhancement Enhancement to performance, inner workings or existent features

Comments

@jmbuhr
Copy link

jmbuhr commented Jul 5, 2024

This issue is here to gauge interest for a PR.

Issue

Telescope ships with built-in pickers for common lsp actions such as got-to-definition and list document symbols. However, those have to be manually bound to keys and are disconnected from the nvim builtin functions like vim.lsp.buf.definition.
As such they have to do some of the work that would already be done by the vim.lsp.buf.<method> functions, such as creating the params for the lsp request and sending the request to the buffers. Optimally, telescope would only have to inject itself into one part of the lsp workflow: the handling of the response, which can easily be configured by assigning to vim.lsp.handlers[<method>].

I do have a secondary motif for this PR: my plugin otter.nvim acts as a language server to provide lsp features for code embedded in other code, like for example python code chunks in markdown(-based) documents or SQL queries in rust code. It does so by sending a modified request to other language servers attached to hidden buffers behind the scenes. It also brings its own handlers to modify the response before passing it on to the default Neovim handler for that type of request (vim.lsp.handlers):

stateDiagram-v2
otterls : otter-ls
params : modified request params
ls : ls attached to otter buffer 🦦1
handler: otter-ls handler
defaultHandler: default handler of nvim
request --> otterls
otterls --> params
params --> ls
ls --> response
response --> handler
handler --> defaultHandler
Loading

Integrating this with telescope is a common feature request (or confusion) I get. But because the current pickers send their own requests manually with vim.lsp.buf_request and inject their own handler, the handler specified by the language server is overwritten (The hierarchy for the handler used by buf_request goes: 1) explicitly set 2) handler provided by the server 3) handler provided by Neovim).

Solution

One solution is to take the relevant pieces from the telescope.builtin.__lsp module (https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/builtin/__lsp.lua) and use it to expose a version of the picker that acts purely as a lsp response handler. I made an example implementation here: https://github.com/jmbuhr/quarto-nvim-kickstarter/blob/main/lua/misc/handlers.lua
(the code is almost the same except that the handler part has been pulled out of the individual pickers).
This can then be used like such:
https://github.com/jmbuhr/quarto-nvim-kickstarter/blob/6455ad9123041f11e3294508db8f216223e2ca8a/lua/plugins/lsp.lua#L118-L122

vim.lsp.handlers[ms.textDocument_definition] = handlers.telescope_handler_factory(ms.textDocument_definition, "Definition")

So overall this is not only useful for integrating with other plugins, but also to remove the need for a different keybinding.
Please let me know what you think :)

Open question

My solution still has one open question: It is not currently using user-defined defaults for opts. Is there a way of accessing those from a normal function?

@jmbuhr jmbuhr added the enhancement Enhancement to performance, inner workings or existent features label Jul 5, 2024
@jamestrew
Copy link
Contributor

Hey sorry for the late response.
This is an interesting subject and an area I've been curious about for a while as well.

Can you clarify this for me? When you say

handler provided by the server

do you mean like via lspconfig (or personal config)? The servers themselves aren't setting vim lsp handlers are they?

because the current pickers send their own requests manually with vim.lsp.buf_request and inject their own handler, the handler specified by the language server is overwritten

This is actually something we've been tracking and I've briefly taken a look at a while back but didn't have any great ideas.
#2768
#3015

So by using something like your telescope_handler_factory and binding telescope picker functions as lsp handler, we can resolve these issues and help with otter.nvim (sounds really cool btw, need to give it a try)? If so, I'm definitely interested.

My solution still has one open question: It is not currently using user-defined defaults for opts. Is there a way of accessing those from a normal function?

We currently merge and bind opts set during telescope setup with those passed at picker call time via this function here

local apply_config = function(mod)
for k, v in pairs(mod) do
mod[k] = function(opts)
local pickers_conf = require("telescope.config").pickers
opts = opts or {}
opts.bufnr = opts.bufnr or vim.api.nvim_get_current_buf()
opts.winnr = opts.winnr or vim.api.nvim_get_current_win()
local pconf = pickers_conf[k] or {}
local defaults = (function()
if pconf.theme then
return require("telescope.themes")["get_" .. pconf.theme](pconf)
end
return vim.deepcopy(pconf)
end)()
if pconf.mappings then
defaults.attach_mappings = function(_, map)
for mode, tbl in pairs(pconf.mappings) do
for key, action in pairs(tbl) do
map(mode, key, action)
end
end
return true
end
end
if pconf.attach_mappings and opts.attach_mappings then
local opts_attach = opts.attach_mappings
opts.attach_mappings = function(prompt_bufnr, map)
pconf.attach_mappings(prompt_bufnr, map)
return opts_attach(prompt_bufnr, map)
end
end
if defaults.attach_mappings and opts.attach_mappings then
local opts_attach = opts.attach_mappings
opts.attach_mappings = function(prompt_bufnr, map)
defaults.attach_mappings(prompt_bufnr, map)
return opts_attach(prompt_bufnr, map)
end
end
v(vim.tbl_extend("force", defaults, opts))
end
end
return mod
end
-- We can't do this in one statement because tree-sitter-lua docgen gets confused if we do
builtin = apply_config(builtin)

Maybe one downside to binding telescope pickers to lsp handlers is that dynamically passing telescope options would probably be not possible (eg. calling vim.lsp.buf.definition with differing telescope options). Even if it's not possible, I don't think this is a huge deal and probably not something many people do.

jmbuhr added a commit to jmbuhr/otter.nvim that referenced this issue Jul 22, 2024
handlers instead of replacing the entire handler server-side

refs nvim-telescope/telescope.nvim#3196
@jmbuhr
Copy link
Author

jmbuhr commented Jul 22, 2024

After your question I looked at my code again with fresh eyes and realized a had indeed muddied the waters between client and server when I wrote that part of otter. The handlers I was creating where indeed not for the server, but the client part of otterls (in vim.lsp.start(handlers = ...)). So my above statement

(The hierarchy for the handler used by buf_request goes: 1) explicitly set 2) handler provided by the server 3) handler provided by Neovim)

Should have been

(The hierarchy for the handler used by buf_request goes: 1) explicitly set 2) handler provided as part of the client when starting the client connecting to the server 3) handler provided by Neovim)

After taking my hands off of the handlers of the client for otterls that we start in the commit just linked in this thread and instead injecting the otter handlers in a callback in between the response and the actual handler that can come from anywhere (default nvim, user-passed, telescope), otterls now works out oft the box with telescope!

So, for compatibility with otter there is nothing to be done on telescope's side anymore, though the telescope as lsp handlers implementation remains an interesting prospect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to performance, inner workings or existent features
Projects
None yet
Development

No branches or pull requests

2 participants