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

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: clangd/ASTManager.cpp:166
 tooling::CompilationDatabase *
 ASTManager::getOrCreateCompilationDatabaseForFile(StringRef Uri) {
   auto &I = CompilationDatabases[Uri];
----------------
I think we should rename the `Uri` parameter here to `File`.


================
Comment at: clangd/ASTManager.cpp:178
 std::unique_ptr<clang::ASTUnit>
 ASTManager::createASTUnitForFile(StringRef Uri, const DocumentStore &Docs) {
   tooling::CompilationDatabase *CDB =
----------------
I think we should rename the `Uri` parameter here to `File`.


================
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") {
----------------
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.


================
Comment at: clangd/Protocol.cpp:167
     if (KeyValue == "uri") {
-      Result.uri = Value->getValue(Storage);
+      Result.uri = Uri::parse(Value->getValue(Storage));
     } else if (KeyValue == "languageId") {
----------------
Same thing: I think that the clients should be responsible of converting `Result.uri` to a filename.


================
Comment at: clangd/Protocol.h:32
 
+struct Uri {
+  static std::string parse(llvm::StringRef uri);
----------------
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.


================
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),
----------------
By the way, what does this do? I'm not a typescript vs code guru.


https://reviews.llvm.org/D31401