-
Notifications
You must be signed in to change notification settings - Fork 5.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
fix(lsp): normalize windows file URLs properly #10034
Conversation
ok, I have test this under windows and it resolves the issue... |
It's not quite clear to me what the purpose of normalizing is here. I would suppose that So if I had written:
It'd import from:
|
4043025
to
e29171d
Compare
@piscisaureus thanks for the feedback. I have taken a different approach, which should be far more foolproof. File URLs are transformed into file paths and then back into file URLs. This ensures that the URL will be normalized in a way that matches Rust's behaviour. I tested it again on windows and it fixes both the linked issues, but should be able to be generically applied to any client URL. |
This looks good! Would be great to have a unit test. |
There are two unit tests, one for a Windows system (which ensures the drive letter get recapitalised) and one for non-windows which deals just with percent encoding of parts of the path. Did you mean some sort of integration test? Those are complicated in this case because of the need to write out real files, mangle them, craft the messages and then send them in and deal with the nature of windows versus non-windows. |
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.
LGTM
Sorry, I must have overlooked the tests that were already there. |
Fixes #9744
Fixes denoland/vscode_deno#386