sammccall added a comment. This looks OK so far, where is it going? It doesn't make sense to put URIs in if we only ever use `file:`. I guess others will be produced by some kind of extension point on SymbolCollector. That will specify URI schemes we should try, allow you to replace the whole `toFileURI`, or something else?
Unfortunately there's a bunch of `Uri`s here, where the existing code uses `URI`... ================ Comment at: clangd/index/Index.h:27 + // The URI of the source file where a symbol occurs. + llvm::StringRef FileUri; // The 0-based offset to the first character of the symbol from the beginning ---------------- nit: FileURI? (The other style is OK too, though I personally find it harder to read. But the class is `URI` and we should be consistent) ================ Comment at: clangd/index/SymbolCollector.cpp:28 +// current working directory of the given SourceManager if the Path is not an +// absolute path. If failed, this combine relative paths with \p FallbackDir to +// get an absolute path. ---------------- this combines or better, "resolves relative paths against \p FallbackDir" ================ Comment at: clangd/index/SymbolCollector.cpp:33 // the SourceManager. -std::string makeAbsolutePath(const SourceManager &SM, StringRef Path, - StringRef FallbackDir) { +std::string toFileUri(const SourceManager &SM, StringRef Path, + StringRef FallbackDir) { ---------------- also URI here, and below ================ Comment at: clangd/index/SymbolCollector.cpp:201 + std::string Uri; + S.CanonicalDeclaration = GetSymbolLocation(ND, SM, Opts.FallbackDir, Uri); ---------------- while here, would you mind changing GetSymbolLocation -> getSymbolLocation? ================ Comment at: clangd/index/SymbolYAML.cpp:51 IO.mapRequired("EndOffset", Value.EndOffset); - IO.mapRequired("FilePath", Value.FilePath); + IO.mapRequired("FileUri", Value.FileUri); } ---------------- more URI Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits