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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
