hokein marked an inline comment as done.
hokein added inline comments.
================
Comment at: clangd/URI.h:66
+ ///
+ /// If the returned string is non-empty, clangd will use it directly when
+ /// doing include insertion; otherwise we will fall back to the clang to
----------------
sammccall wrote:
> does this include <quotes>? it probably should, if we're allowing schemes to
> customize how includes are spelled.
Yes, the returned string is a literal string quoted with either <> or "".
================
Comment at: clangd/URI.h:69
+ /// calculate the include path from the resolved absolute path.
+ static llvm::Expected<std::string> includePath(const URI &U);
+
----------------
sammccall wrote:
> i'd avoid the name "include path" because it can be confused with a) the set
> of directories passed to -I and b) the filesystem path to the file to be
> included.
> Suggest includeString or so
Renamed it to `spellingInclude`.
================
Comment at: unittests/clangd/ClangdTests.cpp:964
EXPECT_TRUE(Inserted("<y.h>", PreferredHeader, "\"Y.h\""));
+ auto TestURIHeader = URI::create("/abc/test-root/x/y/z.h", "unittest");
+ EXPECT_TRUE(static_cast<bool>(TestURIHeader));
----------------
sammccall wrote:
> this relies on ClangdTests and URITests being linked together, which we chose
> to avoid last time this came up. Just define a scheme here?
> (This is one place where putting a field on URI would have been simpler)
I think we probably want a scheme for unittest purpose (we also have one for
lit test), but this case only needs a trivial one, added here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45426
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
