Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

Ability to use vim.lsp.handlers for invokation #16

Open
xanderio opened this issue Oct 3, 2021 · 7 comments
Open

Ability to use vim.lsp.handlers for invokation #16

xanderio opened this issue Oct 3, 2021 · 7 comments

Comments

@xanderio
Copy link

xanderio commented Oct 3, 2021

Is there a reason that this plugin doesn't provide a handler for registration with vim.lsp.handlers['textDocument/codeAction']?

An example of a plugin that provides this is nvim-lsputils.

@weilbith
Copy link
Owner

weilbith commented Oct 4, 2021

Hey.
Sorry for the delay.
I think you have a good point. To be honest I haven't thought about it probably because personally I had some issues with the design approach NeoVim took with its hooks/handlers. Caused me many issues.. But if I'm not mistaken they tried to improve that. Anyway I think it should be fine to provide users with a handler function they can assign themselves.
I would not do this automatically for multiple reasons. First the user should have the option and don't just get his possible handler overwritten (for what ever reason someone likes to have both). Second this makes lazy loading this plugin way more challenging. Third, I'm currently focusing on working with diverse language client implementations (e.g. CoC) and also diverse front-end/menu plugins. Especially for the client part, the handler approach might not be possible at all. So the custom command will probably don't go away. 😅

If you have some additional arguments/opinions I would love to hear them. ☺️

@mjlbach
Copy link

mjlbach commented Nov 19, 2021

To be honest I haven't thought about it probably because personally I had some issues with the design approach NeoVim took with its hooks/handlers.

What's the issue?

It's fine for plugins to call their own request/pass their own handler.

@weilbith
Copy link
Owner

weilbith commented Nov 23, 2021

What's the issue?

It's fine for plugins to call their own request/pass their own handler.

I think in first place because there can be only one handler. Plugins started to just set a handler, making them incompatible with other plugins, breaking configs etc. pp.
I might have preferred a middleware like architecture. Seems more like a typical design approach for such a problem. Though after all I'm not even sure any of these solutions is necessary here (not even handlers).
But I could have taken part in the discussions while this got developed. It is stupid to complain about it now. You must not use handlers to use native LSP, especially as a plugin developer. So I think this is totally fine.

@mjlbach
Copy link

mjlbach commented Nov 23, 2021

I think in first place because there can be only one handler.

This makes sense, because the handler is supposed to be the default interaction for a given request/response pair. I'm not sure the scenario in which it makes sense for multiple plugins to control the response. Users are also free to wrap handlers with whatever dispatch logic then way (handlers could forward arguments to other handlers based on anything passed in ctx.

You must not use handlers to use native LSP, especially as a plugin developer. So I think this is totally fine.

I don't agree with this, I'm not sure in what situations it makes sense for a user to use multiple plugins for the same handler.

@weilbith
Copy link
Owner

I do really think this is not the place to discuss it. So finally just as an example: diagnostics - do virtual text, do signs, publish location list, status list update, ... This code here: vim.lsp.handlers["textDocument/publishDiagnostics"] = vim.lsp.with(vim.lsp.diagnostic.on_publish_diagnostics, { ... }) is a symptom of it.

After all I'm fine with it. It is a valid approach and you have arguments for it. And I think for most events this is fine. All good. And if the feature here is desired by multiple users I can restructure some code to make it work within this plugin. I'm also totally fine to accept PRs for it.

After all I regret to have published this plugin in the state it is. As my post on Reddit stated it wasn't ready. It probably requires major refactoring. Especially to work with other LSP clients despite the inbuild one. And ofc the diff generation is a nightmare. The code here is not high quality. And it bugs me. Though I struggle to find the time working on it.

@bnwa
Copy link

bnwa commented Feb 26, 2023

I do really think this is not the place to discuss it. So finally just as an example: diagnostics - do virtual text, do signs, publish location list, status list update, ... This code here: vim.lsp.handlers["textDocument/publishDiagnostics"] = vim.lsp.with(vim.lsp.diagnostic.on_publish_diagnostics, { ... }) is a symptom of it.

After all I'm fine with it. It is a valid approach and you have arguments for it. And I think for most events this is fine. All good. And if the feature here is desired by multiple users I can restructure some code to make it work within this plugin. I'm also totally fine to accept PRs for it.

After all I regret to have published this plugin in the state it is. As my post on Reddit stated it wasn't ready. It probably requires major refactoring. Especially to work with other LSP clients despite the inbuild one. And ofc the diff generation is a nightmare. The code here is not high quality. And it bugs me. Though I struggle to find the time working on it.

Just as an aside, I'm taken aback by how little there are of publicly re-usable functions for basic tasks for extending LSP behavior. Just in the case of populating a vim.ui.select with all attached client code actions as well as the out-of-spec code actions for tsserver, you either have to write a bunch of code that already exists in neovim's runtime lua but is marked private or, like a number of plugins I've looked into which expand the code action UI or behavior, you just copy and paste from Neovim's runtime lua code for things every code action plugin developer is going to use, e.g. get all the attached clients' code actions, open a select UI, respond to the user selection, respond to the LSP code action co-routine, etc

@mjlbach
Copy link

mjlbach commented Feb 26, 2023

I don't use or work on neovim anymore, but it was always the plan to gradually expose internal functions that were required for plugin developers, or alternatively, improve the existing publicly available apis (like we did via the introduction of vim.ui.select) to allow better composition of plugins, and decrease the boiler plate required where applicable.

Marking functions private was done because there was a trend of plugins (especially lsp-saga) using functions that were supposed to be internal without communicating that there was a need for a publicly available api, which meant that when we wanted to do any large internal refactors I had to do a multi-week coordination with various plugin authors and still would end up breaking XYZ workflow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants