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
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void
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
protocol.test)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
