ilya-biryukov added a comment.
Only the naming of fields left.
Also, please make sure to run `git-clang-format` on the code before submitting
to make sure it's formatted properly.
================
Comment at: clangd/ClangdLSPServer.cpp:331
+ if (!H) {
+ replyError(ErrorCode::InternalError,
+ llvm::toString(H.takeError()));
----------------
NIT: use `return replyError` to be consistent with other methods.
================
Comment at: clangd/XRefs.cpp:354
+
+ return Name;
+}
----------------
simark wrote:
> ilya-biryukov wrote:
> > We should call `flush()` before returning `Name` here. The
> > `raw_string_ostream` is buffered.
> I replaced it with `Stream.str()`.
Also works, albeit less efficient (will probably do a copy where move is
enough).
================
Comment at: unittests/clangd/XRefsTests.cpp:262
+ struct OneTest {
+ StringRef input;
+ StringRef expectedHover;
----------------
simark wrote:
> ilya-biryukov wrote:
> > NIT: LLVM uses `UpperCamelCase` for field names.
> Ok, I fixed all the structs this patch adds.
Argh, sorry that I didn't mention it before, but we have an exception for
structs in `Procotol.h`, they follow the spelling in the LSP spec (i.e. should
be `lowerCamelCase`).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
