Work around Uri round-tripping issue in VS Code for sourcekit-lsp scheme
#1026
Conversation
src/sourcekit-lsp/uriConverters.ts
Outdated
|
|
||
| // Regular expression to decompose a URI, copied from | ||
| // https://github.com/microsoft/vscode/blob/1647dde06cd9b610e2576f09f68101c31c00e574/src/vs/base/common/uri.ts#L80 | ||
| const _decodeUriRegexp = /^(([^:/?#]+?):)?(\/\/([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?/; |
There was a problem hiding this comment.
This works, but is it possible to use Node’s URL implementation here over the regex? All the sourcekit lsp Uris should be compatible with the initializer.
You’d have to build the authority out of the urls host and optional port.
There was a problem hiding this comment.
Internally VSCode uses decodeURIComponent
There was a problem hiding this comment.
Good suggestion with using Node’s URL type to do the parsing. I updated the PR to use Node’s URL parser.
|
@ahoppen Thanks for providing this workaround. Did you test if this works without the sourcekit-lsp's workaround which you made earlier? |
ahoppen
force-pushed
the
uri-coding
branch
2 times, most recently
from
57af010 to
ab7eedb
Compare
Yes, I did and nested macro expansions worked. |
|
@plemarquand Could you take another look at my new implementation that’s based on Node’s |
There was a problem hiding this comment.
@ahoppen Sorry I didn't come back to this one. LGTM, though I didn't realize quite how much vscode.URI and URL diverged.
… scheme
`vscode.uri` fails to round-trip URIs that have both a `=` and `%3D` (percent-encoded `=`) in the query component.
```ts
vscode.Uri.parse("scheme://host?outer=inner%3Dvalue").toString() -> 'scheme://host?outer%3Dinner%3Dvalue'
vscode.Uri.parse("scheme://host?outer=inner%3Dvalue").toString(/*skipEncoding*/ true) -> 'scheme://host?outer=inner=value'
```
The SourceKit-LSP scheme relies heavily on encoding options in the query parameters, eg. for Swift macro expansions and the values of those query parameters might contain percent-encoded `=` signs.
To work around the round-trip issue, use the URL type from Node.js to parse the URI and then map the URL components to the Uri components in VS Code. There’s still some mapping logic needed because the two don't agree on whether to include eg. `?` as part of the query or not, but it’s fairly straightforward.
|
Yeah, I think the key takeaway here is that |
…s to LSP requests" This reverts commit b2c17c7. The workaround isn’t needed anymore because we have a proper fix in the VS Code extension: swiftlang/vscode-swift#1026
vscode.urifails to round-trip URIs that have both a=and%3D(percent-encoded=) in the query component.The SourceKit-LSP scheme relies heavily on encoding options in the query parameters, eg. for Swift macro expansions and the values of those query parameters might contain percent-encoded
=signs.To work around the round-trip issue, don't percent-decode any components of the URI and then use
toString(/*skipEncoding*/ true)to serialize it into a string again.Technically, we should percent-decode authority and path but if we do that, we also need to percent-encode it in
code2Protocol, which means that we can't usevscode.Uri.toStringanymore and would need to write our own encoding code. Since SourceKit-LSP currently doesn't use any characters in authority and path that need percent encoding for thesourcekit-lsp:scheme, we can just not do any decoding here.