autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 272 forks source link

Feature Request: new autocmd event `LanguageClientInitialized` for registering handlers before anything happens? #1176

Closed fanzeyi closed 3 years ago

fanzeyi commented 3 years ago

Is your feature request related to a problem? Please describe.

In our language server implementation, we have some customize events that I'd like to register handlers for since it would crash if the language client does not handle these events at all. The tricky part is that it sends some of these custom events at the very beginning of a session.

My understanding is that we can only register handler after the language server is started, so I came up with the following:

augroup LanguageClient_config
  autocmd User LanguageClientStarted call RegisterEventHandlers()
augroup END

However, this doesn't really work since by the time languageClient-neovim triggers the LanguageClientStarted event, the server already sent some of the custom events and crashed since they are not handled.

Describe the solution you'd like

To solve this issue, I tried to add a new event LagnuageClientInitialized right after the server is started and this does register the custom event handlers in time:

diff --git a/src/language_server_protocol.rs b/src/language_server_protocol.rs
index b65868e..d2ac969 100644
--- a/src/language_server_protocol.rs
+++ b/src/language_server_protocol.rs
@@ -3885,6 +3885,10 @@ impl LanguageClient {
         self.sync_settings()?;
         info!("settings synced");

+        self.vim()?
+            .rpcclient
+            .notify("s:ExecuteAutocmd", "LanguageClientInitialized")?;
+
         let command = self
             .get(|state| state.server_commands.get(&language_id).cloned())?
             .ok_or_else(|| {

I wonder if this is acceptable to be merged to this repository. Thanks!

martskins commented 3 years ago

I think something like that could be merged yeah, although I would change the name of the autocmd, as that could easily be mistaken for LSP initialized, which happens after that.

Out of curiosity, are you using LanguageClient#registerHandlers() inside that RegisterEventHandlers function?

I think a better strategy would be to include a dictionary of handlers in the server config, so that different handlers can be configured for each server independently, and also so that the handlers are registered before the server starts. This is obviously a bigger change, but now that we allow for more detailed server config it should be rather straight forward.

fanzeyi commented 3 years ago

I think something like that could be merged yeah, although I would change the name of the autocmd, as that could easily be mistaken for LSP initialized, which happens after that.

Great! Maybe something like LanguageClientCreated is a better name?

Out of curiosity, are you using LanguageClient#registerHandlers() inside that RegisterEventHandlers function?

Yep.

... so that different handlers can be configured for each server independently, and also so that the handlers are registered before the server starts.

Agree. This should be a better solution. nvim_lsp does allow you to configure a custom handler in configuration directly without registering after the server is started. Having something like that in LanguageClient-neovim would be quite nice. Is there an issue tracking this?

martskins commented 3 years ago

There's no issue tracking this, nope. If you are happy to create one I can try to open a PR for this in the next few days. Or if anyone else wants to tackle this one that would also be great!

martskins commented 3 years ago

Closing this via #1177