[PATCH] D31401: [clangd] Extract FsPath from file:// uri

[Stanislav Ionascu via Phabricator via cfe-commits]

stanionascu marked 5 inline comments as done.
stanionascu added inline comments.


================
Comment at: clangd/Protocol.cpp:61
     if (KeyValue == "uri") {
-      Result.uri = Value->getValue(Storage);
+      Result.uri = Uri::parse(Value->getValue(Storage));
     } else if (KeyValue == "version") {
----------------
krasimir wrote:
> Hm, seems to me that here the uri should be kept as-is and the clients of `Result.uri` should convert it to file when appropriate. Otherwise it's confusing.
Implemented the "URI::parse" to store both representations.


================
Comment at: clangd/Protocol.h:32
 
+struct Uri {
+  static std::string parse(llvm::StringRef uri);
----------------
krasimir wrote:
> It's a little confusing: the `parse` method turns an `uri` to a `file` and the `unparse` method does the opposite. Maybe we should use more descriptive names, like `uriToFile` and `fileToUri`. This does not seem to be inconsistent with the rest of the protocol since the protocol only cares about uri-s, and file-s are an implementation detail of clangd I guess.
2nd try now, instead of actually storing a string based URI, store the object which both representations (uri and file).


================
Comment at: clangd/clients/clangd-vscode/src/extension.ts:28
+        uriConverters: {
+            // FIXME: implement percent decoding in clangd
+            code2Protocol: (uri: vscode.Uri) : string => uri.toString(true),
----------------
krasimir wrote:
> By the way, what does this do? I'm not a typescript vs code guru.
Extended the FIXME, with the description, e.g. when vscode (on windows) sends the uri to clangd, it's represented as "file:///c%3A/path/tofile" which leads to:
- incorrect detection of the compilation database location.
- cannot find the actual unit in the database.

uri.toString(true) - skips the percent encoding and sends the uri parameter to clangd as "file:///c:/path/tofile"


https://reviews.llvm.org/D31401