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
