Skip to content

fix: match nested settings through symlinks and .., and follow vcs for discovery - #936

Open
psteinroe wants to merge 5 commits into
perf/per-connection-snapshot-loadingfrom
fix/nested-config-paths
Open

psteinroe wants to merge 5 commits into
perf/per-connection-snapshot-loadingfrom
fix/nested-config-paths

Conversation

@psteinroe

Copy link
Copy Markdown
Collaborator

Stacked on #921 → #915 → #908 → #902 → #895 → #881. Must merge after #881.

The path-matching edge cases from the release review (C19). Settings are picked by comparing a file's path with the workspace and nested folders as strings, which went wrong in three cases that are clearly bugs.

.. picked the folder it leaves. /repo/db1/../db2/a.sql starts with /repo/db1, so it got db1's settings (and database). The CLI produces such paths from check db1/../db2/a.sql, and editors can too. Paths are now normalized lexically (./.. removed, without touching the disk) before the settings lookup in the workspace and before the LSP picks the project. normalize_path moved from pgls_workspace::configuration to pgls_fs for that.

Symlinked workspace folders. With a folder opened as /home/a/link (a symlink to /data/repo) and a file opened by its real path, e.g. from a "go to definition" or a terminal link, the file matched no folder and got the defaults of a file outside every folder (since #902). The workspace now remembers the real path of each project folder and maps paths below it back onto the folder; the LSP does the same when picking the project. On Windows the \\?\ prefix that canonicalize adds is stripped.

.gitignore with vcs.enabled: false. The nested configuration search always honoured .gitignore, while the settings only use it with vcs.enabled and vcs.useIgnoreFile (and this repo's own config sets enabled: false). With VCS off, a config in a gitignored folder was silently not loaded, although the files there were checked. FindFiles gets vcs_ignore_files, which the search sets the way the settings would; .ignore files are always honoured.

Documented, not changed (docs/configuration.md):

  • The reverse symlink case: a folder opened by its real path and a file opened through a symlink. That would need a canonicalize per file and request.
  • Case-insensitive file systems: /Users/a/Repo and /Users/a/repo are different folders to the server. Editors send the spelling of the workspace folder, so this only bites when paths are typed by hand.
  • A nested configuration inside the parent's migrationsDir replaces the migrations settings too (nothing is inherited), so its files aren't migrations unless it sets migrations itself or extends the parent. This follows from the no-inheritance design of feat: apply the nearest configuration file to each file (nested configs) #881.

Tests

  • pgls_workspace: files_below_the_real_path_of_a_symlinked_folder_use_its_settings (Unix), and .. cases in files_use_the_nearest_configuration_file.
  • LSP: files_opened_by_their_real_path_belong_to_a_symlinked_folder (Unix).
  • pgls_fs: honours_gitignore_files_only_if_asked.
  • CLI: nested_configurations_ignore_gitignore_without_vcs.

Risks

…r discovery

A file opened by its real path gets the settings of a workspace folder
opened through a symlink, and `..` in a path no longer selects the
settings of the folder it leaves. The nested configuration search only
honours .gitignore when the root configuration does (vcs.enabled and
vcs.useIgnoreFile). Document the remaining path edge cases.

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