This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.
ClosedPublic

Authored by ioeric on Feb 22 2018, 10:07 AM.

Diff Detail

Event Timeline

ioeric created this revision.Feb 22 2018, 10:07 AM
sammccall accepted this revision.Feb 22 2018, 10:27 AM

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

clangd/Protocol.h
301

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.

test/clangd/want-diagnostics.test
5 ↗(On Diff #135457)

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)

This revision is now accepted and ready to land.Feb 22 2018, 10:27 AM
ioeric updated this revision to Diff 135463.Feb 22 2018, 10:42 AM
ioeric marked 2 inline comments as done.

Addressed review comments. Removed test.

This revision was automatically updated to reflect the committed changes.