LuaLS / LuaLS/lua-language-server
The "workspace/didChangeConfiguration" handler ignores message params
- Dominant language
- Lua
- Stars
- 4.4k
- Forks
- 442
- PR merge metrics
- No merged PRs in 30d
Description
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](https://github.com/dense-analysis/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:
1. The folder-scoped configuration(s), pulled from the client via a `workspace.configuration` request
2. The `.luarc.json(c)` file in each folder
3. The global / unscoped configuration (`scope.fallback` in the source) pulled from the client via a `workspace.configuration` request
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](https://github.com/dense-analysis/ale/issues/4591).
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:
```jsonc
{
"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](https://github.com/tmillican/lua-language-server), and I'd be happy to make a PR if this proposal sounds reasonable.
Contributor guide
Research direction
Locate the workspace/didChangeConfiguration handler and the config loader entry point named loadClientConfig(), then trace how scope.fallback is populated. Done means params.settings is applied between the folder-scoped and global configuration steps, while preserving the existing behavior for missing or unexpected settings and --configPath.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- lua
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100