Skip to content

fix(lsp): watch every workspace folder, the user config and extended files - #908

Open
psteinroe wants to merge 4 commits into
fix/nested-config-isolationfrom
fix/watch-configuration-files
Open

psteinroe wants to merge 4 commits into
fix/nested-config-isolationfrom
fix/watch-configuration-files

Conversation

@psteinroe

Copy link
Copy Markdown
Collaborator

Stacked on #902 → #895 → #881. Must merge after #881.

The configuration watcher was registered for rootUri only (C5), as <rootUri>/**/postgres-language-server.jsonc with the path pasted into the glob unescaped. In a multi-root workspace, or for a folder added later through didChangeWorkspaceFolders, creating, editing or deleting a configuration file was never picked up, and a client that sends workspaceFolders but no rootUri got no watcher at all. The user configuration (#878) and extends targets weren't watched either, and nothing told the user that the user configuration was being applied (C16).

What changed

  • One watcher per workspace folder (or the root URI when there are no folders): RelativePattern { baseUri: <folder>, pattern: "**/postgres-language-server.jsonc" } when the client announces relativePatternSupport, else a string glob with the path escaped (*?[{} in brackets, / as separator on Windows). They are registered again after every reload, so added and removed folders are covered.
  • Also watched: the configuration file above a workspace folder that applies to it, the explicit configurationPath, the user configuration paths (also before one exists), and every file a loaded root or nested configuration extends. A change of any of them goes through the debounced reload from perf(workspace): bound nested configuration discovery #895. When a configuration fails to load, the files it extended before stay watched, so fixing it is picked up.
  • Announcement: when the user configuration applies, the LSP logs Using the user configuration at <path> for <folder>, which has no configuration file. (INFO window/logMessage) and the CLI prints Using the user configuration at <path> to stderr, so it stays out of --reporter json and format --stdin-file-path output.
  • apply_extends now returns the files it extended, and LoadedConfiguration carries them plus is_user_configuration.

Tests

  • LSP the_configuration_files_of_every_workspace_folder_are_watched: no rootUri, two folders, both with and without relativePatternSupport; after adding three and removing one, the new registration covers three and not one.
  • LSP a_change_of_an_extended_file_reloads_the_configuration: editing ../shared/base.jsonc outside the workspace folder changes the severity of an open document.
  • LSP the_user_configuration_and_extended_files_are_watched: the registration lists both.
  • CLI check_announces_the_user_configuration (Unix, via XDG_CONFIG_HOME): the line is on stderr, not in the JSON report.

Not covered

  • Only ] is left unescaped in string globs; LSP glob syntax has no escape for it, and a lone ] is literal.
  • A missing extends target can't be watched, because the load fails before it is known; the configuration file itself still is.

…files

Register one watcher per workspace folder (a relative pattern if the
client supports it, else an escaped glob) and register them again when
the folders change. Also watch the configuration file above a folder,
an explicit configuration path, the user configuration and every file a
loaded configuration extends. Announce in the CLI and the LSP log when
the user configuration applies.
…on-files

Also use a parking_lot lock for the watched configuration files.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant