[PATCH] D35894: [clangd] Code hover for Clangd

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 15 08:41:12 PST 2018


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





More information about the cfe-commits mailing list