sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice,  this will be useful for at least a couple of editor integrations.

Comment at: clangd/Protocol.h:301
+  /// If this is not set, diagnostics will be generated for the current version
+  /// or a subsequent one.
Nit: a little weird to lead with the missing case. Suggest rephrase as:

Forces diagnostics to be generated, or to not be generated. for this version of 
the file.
If not set, diagnostics are eventually consistent: either they will be provided 
for this version
or some subsequent one.

Comment at: test/clangd/want-diagnostics.test:5
 main() {}"}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
I think this test doesn't test anything useful because the check lines are not 
sequenced with respect to the input.

I'm not sure a lit test is that useful here, the actual logic is already unit 
tested. If we really want to test the ClangdLSPServer change, a unit test might 
be easier to get right, but personally I'd be happy enough leaving the logic 
untested (and maybe throwing a wantDiagnostics into one of the updates in 

  rCTE Clang Tools Extra

cfe-commits mailing list

Reply via email to