ilya-biryukov added inline comments.

================
Comment at: unittests/clangd/XRefsTests.cpp:53
 
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+  void onDiagnosticsReady(
----------------
malaperle wrote:
> ilya-biryukov wrote:
> > NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h` instead.
> It looks like IgnoreDiagnostics from Compiler.h implements DiagnosticConsumer 
> from Compiler.h, and not DiagnosticsConsumer from ClangdServer.h so that 
> wouldn't work.
You're right, I got confused, sorry. Disregard my comment.


================
Comment at: unittests/clangd/XRefsTests.cpp:245
+  const char *SourceContents = R"cpp(
+  #include "$2^invalid.h"
+  #include "^foo.h"
----------------
malaperle wrote:
> ilya-biryukov wrote:
> > Could we also add tests for corner cases: cursor before opening quote, 
> > cursor after the closing quote, cursor in the middle of `#include` token? 
> > (we shouldn't navigate anywhere in the middle of the #include token)
> > cursor before opening quote, cursor after the closing quote
> 
> I assume we don't want to navigate anywhere for these positions? I don't have 
> an opinion.
> 
> > (we shouldn't navigate anywhere in the middle of the #include token)
> 
> It did in CDT and I thought it worked nicely as it made it easier to click on 
> it. You can hold ctrl on the whole line and it underlined it (well except 
> trailing comments). But clients don't handle the spaces nicely (underline 
> between #include and file name) so I thought I'd work on the client first 
> before making the server do it. Anyhow, for now it shouldn't navigate indeed.
> > cursor before opening quote, cursor after the closing quote
>I assume we don't want to navigate anywhere for these positions? I don't have 
>an opinion.
I'd say we should navigate when the cursor is right before the opening quote. 
For the closing quote I don't have any opinions either :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to