ioeric added inline comments.
================
Comment at: clangd/ClangdServer.h:243
+ /// string quoted with <> or "" that can be #included directly.
+ /// \p PreferredHeader The preferred header to be inserted. The has the same
+ /// semantic as \p OriginalHeader. If empty, \p OriginalHeader is used to
----------------
sammccall wrote:
> sammccall wrote:
> > sammccall wrote:
> > > "this has the same semantic as OriginalHeader" contradicts
> > > "[OriginalHeader] may be different from preferredHeader" :-)
> > T think `InsertedHeader` might be clearer about its role.
> I'd suggest being explicit, since this is different to above: "if this is a
> URI, insertInclude will determine how to spell it. If this is a quoted
> string, it will be inserted verbatim".
Bad wording... sorry about that!
================
Comment at: clangd/Headers.cpp:126
+ };
+ if (Included(OriginalHeader) || Included(PreferredHeader))
return llvm::make_error<llvm::StringError>(
----------------
sammccall wrote:
> oops, missed this in the previous commit - this isn't an error condition, but
> a "return empty string" condition.
>
> Can we add a test for this?
There are tests for this in `HeadersTests.cpp`. It's just that in the tests, we
treat both errors and empty insertion as no insertion.
================
Comment at: clangd/Headers.h:23
+/// Returns true if \p Include is literal include like "path" or <path>.
+bool isLiteralInclude(llvm::StringRef Include);
+
----------------
sammccall wrote:
> only used in headers.cpp, make static?
Oops, the same logic appeared in ClangdServer. I wanted to share this but
forgot to...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43510
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
