Fix lsp symlink deduplication #13584
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…te tabs When an LSP server (e.g., pyright) returns a URI for a file that was originally opened via a symlink, it often returns the resolved, canonical URI. Previously, Helix's conversion from this `file://` URI to an internal `helix-core::Uri` used a normalization function that did not fully resolve symlinks. This could lead to a mismatch: 1. You open `/symlink/file.py`. Helix stores its path, canonicalized to `/real/path/to/file.py`. 2. LSP "go to definition" returns URI for `/real/path/to/file.py`. 3. Helix converts this URI. If this conversion doesn't also result in the exact same canonical form as in step 1 (due to not resolving the symlink part if the LSP URI was subtly different but still pointed canonically to the same file, or if the internal URI representation wasn't consistently canonical), Helix might not find the existing document. 4. This would result in Helix opening the same file content in a new tab, treating it as a distinct file. This change modifies `helix-core/src/uri.rs` to use `helix_stdx::path::canonicalize` when converting `file://` URLs to `helix-core::Uri::File`. This ensures that the path stored in the `Uri` is the fully resolved, canonical path. Combined with `Document::set_path` already storing canonical paths for documents, this change ensures that URIs from LSPs and internal document URIs are both in a consistent, canonical form, allowing Helix to correctly identify and reuse existing buffers for symlinked files. I attempted to add an integration test, but I was hindered by compilation timeouts in the testing environment.
This commit addresses an issue where Helix could open duplicate tabs for
the same file if that file was accessed via a symlinked path and an LSP
server (e.g., pyright) returned a fully resolved (canonical) path for
"go to definition" or similar actions. This was particularly problematic
when the entire workspace root was under a symlink.
I've implemented the following changes:
1. **Canonicalize LSP Client Root URIs (`helix-lsp`):**
- I modified `helix-lsp/src/lib.rs` (specifically `find_lsp_workspace`
and `start_client`) and `helix-lsp/src/client.rs`
(`Client::try_add_doc`) to ensure that the LSP client's
`root_path`, `root_uri`, and `workspace_folders` are stored in
their fully canonicalized (symlink-resolved) forms. This ensures
the LSP client operates with a canonical understanding of its
workspace root(s).
2. **Canonicalize Incoming LSP URIs (`helix-core`):**
- I modified `helix-core/src/uri.rs` in the `convert_url_to_uri`
function. When a `url::Url` with a `file://` scheme is converted
to a `Uri::File`, the path is now processed using
`helix_stdx::path::canonicalize` instead of
`helix_stdx::path::normalize`. This ensures URIs from LSP messages
are also in canonical form.
3. **Verified Document Path Storage (`helix-view`):**
- I confirmed that `Document::set_path` (in `helix-view/src/document.rs`)
already uses `helix_stdx::path::canonicalize`. This means
`Document` objects store their paths canonically.
4. **Verified URI Comparisons (`helix-view`):**
- I confirmed that lookups like `Editor::document_by_path` (in
`helix-view/src/editor.rs`) correctly compare canonical paths,
which, due to the above changes, should ensure consistent matching.
These changes collectively ensure that paths/URIs from different sources
(your input, LSP client configuration, LSP messages) are all resolved to
their canonical forms before comparison or use in lookups, preventing
the erroneous opening of duplicate buffers for symlinked files.
I wrote an integration test (`lsp_goto_definition_symlinked_workspace` in
`helix-term/tests/symlink_lsp_workspace_test.rs`) to
specifically cover the symlinked workspace root scenario. However,
persistent compilation timeouts in the testing environment prevented
this test from being run and validated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue I'm seeing:
I open
hx /symlinked/path.pythen go to definition jumps to another fully resolved path, but if I do "hxrealpath $PATH" things then work fine.My project structure: I have
~/projects/fooas my git repo root and helix workspace, but actually~/projectsis a symlink to/data/projects. Go to definition in LSP (pyright) ends up pointing to/data/projectswhich ends up being open in a separate tab