lua-language-server
lua-language-server copied to clipboard
The "workspace/didChangeConfiguration" handler ignores message params
When a client sends a workspace/didChangeConfiguration notification to the server, the server discards the message parameters entirely.
The LSP specification defines the type of the params property for this notification as:
interface didChangeConfigurationParams {
/**
* The actual changed settings
*/
settings: LSPAny
}
LSPAny leaves the door open for a server to handle this data arbitrarily, so I suppose it isn't a bug per se to handle it by discarding the settings . Nevertheless, the comment creates an expectation that the client can send the altered settings in the settings property, and the server will use them to update its configuration.
As near as I can tell, this is how most LSP servers behave, and what most LSP clients are expecting. For instance, this is the behavior of both neovim's built-in LSP client and the ALE plugin for (neo)vim.
Currently, unless --configPath is specified (in which case the notification is ignored), the handler for this notification patches the server configuration from these sources, in this order:
- The folder-scoped configuration(s), pulled from the client via a
workspace.configurationrequest - The
.luarc.json(c)file in each folder - The global / unscoped configuration (
scope.fallbackin the source) pulled from the client via aworkspace.configurationrequest
Notably, 1 and 3 only occur if the client advertised the workspace.configuration capability. If it did not, lua-language-server becomes completely unconfigurable via LSP alone. This is specifically a problem for ALE per this issue.
I would propose that between steps 2 and 3, the server should patch scope.fallback with any settings provided in params.settings, using the same sections polled by the workspace.configuration message. In other words, identical to what is returned from loadClientConfig() in the config loader. Ex:
{
"method": "workspace/didChangeConfiguration",
"jsonrpc": "2.0",
"params": {
"settings": {
"Lua": {
"diagnostics": {
"enable": true,
"globals": ["vim"]
}
// etc.
},
"files.associations" = { /* ... */ },
"editor.semanticHighlighting.enabled" = true
}
}
}
This is consistent with how neovim and ALE -- and probably others -- already try to use the notification, and it won't affect anything for existing clients that don't send params.settings or that send then in some other unexpected format (their params.settings were ignored before and will continue to be ignored after).
I've made a fork with this proposed workflow here, and I'd be happy to make a PR if this proposal sounds reasonable.