MaskRay / ccls

C/C++/ObjC language server supporting cross references, hierarchies, completion and semantic highlighting
Apache License 2.0
3.78k stars 260 forks source link

workspace: Make didChangeConfiguration use its parameter #973

Open rherilier opened 1 month ago

rherilier commented 1 month ago

Hi,

Here is a patch to make workspace/didChangeConfiguration capable of changing the configuration settings as describe in the LSP spec.

any comment/feed-back are welcome

MaskRay commented 3 weeks ago

Do you have a language client that needs this?

rherilier commented 2 weeks ago

eglot (the Emacs' builtin LSP client) does.

It is far better compared to restarting ccls when doing a git rebase or switching between builds.

And it's the last point I have to solve to switch my Emacs config from lsp-mode to eglot :)

Thx for all you work on ccls.

rherilier commented 1 week ago

It seems your are waiting for a more rationale reason. Here I go:

I looked at 3 editors to see how they handle the LSP client part.

First, vscode (obviously) relies on workspace/didChangeConfiguration (w/dCC for short). It emits one right after initialize to send user-defined parameters (that point is not really explicit in specification but as Microsoft is the originator of LSP, we can consider vscode as an implementation reference). ccls works because the extension vscode-ccls restarts ccls instead of emitting a w/DCC.

Second, neovim expects to configure the LSP server with a w/dCC just like vscode. No forced restart available (and no reference to ccls it its code-base).

Last, Emacs (which has 2 LSP clients). The one I want to use (eglot) expects to send LSP server configuration through w/dCC. So, same default behavior as the 2 other editors. No forced restart available even if ccls is detected.

The other client I want to move from (lsp-mode) is design to rely on w/dCC too for all the language server it supports. But to make ccls works with it, an other package (emacs-ccls), which depends on lsp-mode, has to be used and it relies on restarting ccls too (and the user has to explicitly do the restart him/herself). Quite bothersome.

Then I looked at some LSP servers code.

All those with support for w/dCC can handle their configuration through this request.

Clangd can handle configuration through w/dCC too but its configuration capability is quite limited (and it does not even handle correctly its own option set)...

So, we have 4 LSP clients expecting the servers (independently of the programming language) will read their configuration from w/dCC. ccls is really usable by 2 because of the existing extension/package which does a restart (and only one is usable without having to write extra code).

So, my PR aims to bring ccls hot-reconfiguration support to any editor which does not have the kill'n'start that vscode-ccls and emacs-ccls have.

Obviously, updating files or symbolic links at the project's root is not really a durable solution as it implies to add unnecessary complexity inside or outside of the editors.

Best regards

MaskRay commented 2 days ago

Thanks for the explanation.

ccls is really usable by 2 because of the existing extension/package which does a restart (and only one is usable without having to write extra code).

Can you point me to the restart code in the 2 clients? What operations did you do to trigger restart?

When does neovim send didChangeConfiguration?

rherilier commented 21 hours ago

Can you point me to the restart code in the 2 clients?

In vscode-ccls, look at src/serverContext.ts and search for onDidChangeConfiguration to find the function doing the restart (commands.executeCommand('ccls.restart');) and its use.

For Emacs with emacs-ccls, the users have to call lsp-mode's function lsp-workspace-restart (defined in lsp-mode.el) to make effective their new settings.

With my PR merged, emacs-ccls should simply use lsp-mode's function lsp--set-configuration to trigger the settings update (many other lsp-mode's back-ends use it if you search for an example).

What operations did you do to trigger restart?

As stated just above, the function to call is lsp-workspace-restart, and for record, the elisp code I had used looks like:

(progn (setq ccls-initialization-options
             `(:compilationDatabaseDirectory ,current-build-dir))
       (lsp-restart-workspace))

When does neovim send didChangeConfiguration?

It happens in runtime/lua/vim/lsp/client.lua around the line 585 and is in a callback for the LSP request initialize. The code excerpt is:

if next(self.settings) then
  self:notify(ms.workspace_didChangeConfiguration, { settings = self.settings })
end

As I never programmed in lua and don't use neovim, I don't know how to consider the file protocol.lua. Asking the neovim's team would be faster if you want to go further ;)

Best regards