Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canonicalize LSP diagnostics path to resolve symlinks and match the document path #7885

Closed
wants to merge 1 commit into from

Conversation

Danacus
Copy link

@Danacus Danacus commented Aug 9, 2023

Issue

When opening a file through a symlink, diagnostic info from the language server is discarded due to a mismatch between the LSP URL and the canonicalized path in helix_view::Document.

Example

Consider the following project tree (example based on cxx):

gen
├── Cargo.toml
└── src
    ├── lib.rs
    └── syntax -> ../../syntax
syntax
└── mod.rs

where lib.rs contains mod syntax.

If we open gen/src/syntax/mod.rs in helix, the path is canonicalized by get_canonicalized_path and it becomes an absolute path like <...>/syntax/mod.rs, since symlinks are resolved by get_canonicalized_path (in contrast to what is said in the comments in helix-core/src/path.rs, is this intended?). This canonicalized path is then stored in helix_view::Document.

However, when diagnostics are published by the language server, we get a URL that is turned into the path <...>/gen/src/syntax/mod.rs. Symlinks are not resolved and this path is not canonicalized. As a result, there is a mismatch between this path and the canonicalized path in helix_view::Document, and the diagnostics are not sent to the open document.

Proposed Solution (this PR)

Canonicalize the path received from the language server to ensure that it matches the path stored in helix_view::Document.

Please let me know if there are any problems with this PR!

P.S.: I believe this may have been a regression of d04288e, which might have changed the behavior of get_canonicalized_path to resolve symlinks, since dunce::canonicalize relies on std::canonicalize internally (on Linux/UNIX). Either way, canonicalizing the path sent by the language server using the same canonicalization method as done by this PR should resolve the current issue.

- Fixes a problem where the path from PublishDiagnostics does not match the path of the Document if the path contains symlinks.
@pascalkuthe
Copy link
Member

pascalkuthe commented Aug 9, 2023

This will be addressed by #7367 which also adds canonicalization. This PR is basically fully duplicate of that one so I am closing it in favor of that. This would have been (almost) thenright fix tough if we didnt already have an existing PR for it. Thank you for contributing!

For better treatment of symlinks see also #7658

@pascalkuthe pascalkuthe closed this Aug 9, 2023
@pascalkuthe pascalkuthe added the R-duplicate Duplicated issue: please refer to the linked issue label Aug 9, 2023
@Danacus
Copy link
Author

Danacus commented Aug 9, 2023

Oops! I completely missed that PR since I was only searching through existing issues. My bad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R-duplicate Duplicated issue: please refer to the linked issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants