[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1746
+
+                 if (DiagOpts.EmbedFixesInDiagnostics && !CodeActions.empty()) {
+                   Diag.codeActions.emplace(CodeActions);
----------------
kadircet wrote:
> we didn't have the empty check before, let's not introduce it now (i.e. we'll still reply with an empty set of code actions rather than "none" when there are no fixes known to clangd)
yeah, I added this empty check to keep `fixits-embed-in-diagnostic.test` unchanged (we don't emit code-action for diagnostic notes), but I think it is fair enough to change it.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1751-1753
                  auto &FixItsForDiagnostic = LocalFixIts[Diag];
-                 llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+                 llvm::move(std::move(CodeActions),
+                            std::back_inserter(FixItsForDiagnostic));
----------------
kadircet wrote:
> i am not sure why this logic was appending to the vector rather than just overwriting. but we shouldn't receive the same diagnostics from the callback here, am i missing something? so what about just:
> ```
> LocalFixIts[Diag] = std::move(CodeActions);
> ```
Agreed, changed to overwriting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148783/new/

https://reviews.llvm.org/D148783