-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
LSP: Key diagnostics off file path instead of URI #7367
Conversation
f06e53c
to
7bbd2d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
d0e6a0e
to
e48f6a0
Compare
e48f6a0
to
b2ed08a
Compare
b2ed08a
to
fca99c3
Compare
fca99c3
to
2425dc7
Compare
This PR might fix the problem where a directory has a
|
I'm getting this issue on windows as well. Helix version 23.10 |
URIs need to be normalized to be comparable. For example a language server could send a URI for a path containing '+' as '%2B' but we might encode this in something like 'Document::url' as just '+'. We can normalize the URI straight into a PathBuf though since this is the only value we compare these diagnostics URIs against. This also covers edge-cases like windows drive letter capitalization.
Yep this should fix how we find the right document for diagnostics for both windows drive letters and any paths that a language server might URL-encode ( |
2425dc7
to
fbd31db
Compare
Applying this patch in windows brings diagnosis back. Thanks a lot |
URIs need to be normalized to be comparable. For example a language server could send a URI for a path containing
+
as%2B
but we might encode this in something likeDocument::url
as just+
. We can normalize the URI straight into a PathBuf though since this is the only value we compare these diagnostics URIs against. This also covers edge-cases like windows drive letter capitalization.Closes #3267
Also see #7321 (thanks for the pointer on this @pascalkuthe!)