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&id=138018#toc
Repository:
rCTE Clang Tool
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
c
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 {0}",
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", ExecuteCommandParams::CLANGD_APPL
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 wrote:
> > As di
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
cla
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
clangd/ClangdServer.
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 &D : Params.context.diagnostics) {
-auto Edits = getFixIts(Para
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 a
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 Ex
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 t
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 wrote
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: clangd/ClangdLSPServer.cpp
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
_
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
_
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 Cl
16 matches
Mail list logo