[PATCH] D44142: [clangd] Revamp handling of diagnostics.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE327282: [clangd] Revamp handling of diagnostics. (authored by ibiryukov, committed by ). Changed prior to commit: https://reviews.llvm.org/D44142?vs=138004=138018#toc Repository: rCTE Clang Tools

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov updated this revision to Diff 138004. ilya-biryukov added a comment. - Replace equality comparison ops with matchers, they were only used in tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44142 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Ship it! (but let's drop the extra operator== if possible?) Comment at: clangd/ClangdLSPServer.cpp:329 + {"title", + llvm::formatv("Apply FixIt

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:329 + {"title", + llvm::formatv("Apply FixIt {0}", GetFixitMessage(D.message))}, {"command",

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov added inline comments. Comment at: clangd/Diagnostics.h:35 + DiagnosticsEngine::Level Severity; + llvm::SmallVector FixIts; + // Since File is only descriptive, we store a separate flag to distinguish sammccall wrote: > sammccall

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov updated this revision to Diff 137987. ilya-biryukov marked 14 inline comments as done. ilya-biryukov added a comment. - Addressed review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44142 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov updated this revision to Diff 137985. ilya-biryukov added a comment. - Added unit test for toLSPDiags Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44142 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

sammccall added a comment. Sorry, last comments were concurrent with the latest snapshot. Except where noted, the old comments still apply I think. Comment at: clangd/ClangdLSPServer.cpp:317 for (Diagnostic : Params.context.diagnostics) { -auto Edits =

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

sammccall added a comment. Not sure if you're waiting on comments from me: the changes look good. As discussed I still think Fix should be a separate thing with a name because that's how editors treat it, and this is the right layer to decide how to do that mapping. Comment

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov updated this revision to Diff 137977. ilya-biryukov marked 8 inline comments as done. ilya-biryukov added a comment. Addressed review comments. The only thing that's left on my todo-list is a test for toLSPDiags(). Everything else should be ready. Repository: rCTE Clang Tools

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov updated this revision to Diff 137589. ilya-biryukov marked 9 inline comments as done. ilya-biryukov added a comment. This is not final, there are still unadressed comments. - Significantly simplified the implementation by removing the DiagList, etc. - Addressed some of the rest of

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.cpp:329 + {"title", + llvm::formatv("Apply FixIt {0}", GetFixitMessage(D.message))}, {"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}, sammccall

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

sammccall added a comment. Behavior looks good. I think we can do a bit better with (most) fixits - your call on whether it makes sense to do here. As discussed i'd simplify the diagnostic containers until we know there's a strong need. Comment at:

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov added inline comments. Comment at: clangd/Diagnostics.h:28 +/// DiagList. +struct PersistentDiag { + llvm::StringRef Message; This could probably use a better name Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44142

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov added a comment. This revision is still missing tests, but I wanted to get feedback for the overall design and the new diagnostic messages. So sending out for review now. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44142

[PATCH] D44142: [clangd] Revamp handling of diagnostics.

ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, ioeric, sammccall. Herald added subscribers: jkorous-apple, mgorny, klimek. The new implementation attaches notes to diagnostic message and shows the original diagnostics in the message of the note. Repository: rCTE