LuaLS / LuaLS/lua-language-server

The "workspace/didChangeConfiguration" handler ignores message params

Open
#2,899 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.