LuaLS / LuaLS/lua-language-server

The "workspace/didChangeConfiguration" handler ignores message params

Offen
#2,899 2 Kommentare 1 Reaktion 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
Lua
Sterne
4.4k
Forks
442
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

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:

  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.

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Finde den Handler für workspace/didChangeConfiguration und den Einstiegspunkt des Konfigurationsladers namens loadClientConfig(), und verfolge dann, wie scope.fallback befüllt wird. Erledigt ist die Aufgabe, wenn params.settings zwischen den ordnerbezogenen und den globalen Konfigurationsschritten angewendet wird, wobei das bestehende Verhalten für fehlende oder unerwartete settings und --configPath erhalten bleibt.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
lua
Bereich
api, backend
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
45/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.