-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add proper handling of escape characters in LSP URIs #1941
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1941 +/- ##
=======================================
Coverage 92.83% 92.84%
=======================================
Files 355 355
Lines 26073 26115 +42
=======================================
+ Hits 24206 24247 +41
- Misses 1867 1868 +1
☔ View full report in Codecov by Sentry. |
a51bfee
to
3939b2e
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.
Looks good in general; a few comments.
It does look like the Windows installation or at least the interaction with bazel is hosed on the CI, that probably has to be fixed first. |
1486051
to
ec0cb96
Compare
1a903c9
to
8b146b5
Compare
#1945 is merged, so after a rebase, this might compile. |
8b146b5
to
359c480
Compare
…s in URIs The URIs in LSP requests from editors are following RFC 3986 - this commit adds parsing escaped symbols in the paths, such as spaces, colons Signed-off-by: Grzegorz Latosinski <glatosinski@antmicro.com>
Signed-off-by: Grzegorz Latosinski <glatosinski@antmicro.com>
359c480
to
4f4acf5
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.
Looks like we now also need to modify the tests in language server test that use the ScopedTestFile
.
Previously they were just assuming that the output paths are "file://" + module_b.filename()
, but now we will have to look if they are PathToLSPUri(module_b.filename())
.
Signed-off-by: Grzegorz Latosinski <glatosinski@antmicro.com>
9fe3681
to
0258ad2
Compare
Signed-off-by: Grzegorz Latosinski <glatosinski@antmicro.com>
0258ad2
to
277fa0f
Compare
…LSPUri instead of custom URI conversion Signed-off-by: Grzegorz Latosinski <glatosinski@antmicro.com>
The tests are updated now |
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
This PR adds methods for encoding and decoding URIs and filesystem paths in the LSP (so that URIs follow RFC 3986 URI specification).