[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:671
+              // FIMXE: this is tricky
+              llvm::StringRef(LSPDiag.Message)
+                  .starts_with_insensitive(Diag.Message))
----------------
this is tricky indeed and conceptually really hard to achieve inside ClangdServer layer.

what about keeping the cache in ClangdLSPServer, but changing the format? Similar to `TweakRef`, we now have a `DiagRef`, which is ClangdServer-native. So we can keep a cache from `(FilePath, clangd::Diagnostic)` to `clangd::DiagRef`, and pass those `DiagRef`s to `ClangdServer` and make sure we're doing the search for sure on the right domain here?

this also gives us the flexibility to change the definition of a `DiagRef` in the future.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:683
+      // Return immediately if this is a quick-fix-only codeAction request.
+      if (Params.RequestedActionKinds.size() == 1)
+        return CB(std::move(Results));
----------------
we can have quick fix kind tweaks too (e.g. populate switch)


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:714-715
   // Tracks number of times a tweak has been offered.
   static constexpr trace::Metric TweakAvailable(
       "tweak_available", trace::Metric::Counter, "tweak_id");
   auto Action = [Sel, CB = std::move(CB), Filter = std::move(Filter),
----------------
can you also move this counter to global scope and use it in `::codeAction` too?


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:387
+  void codeAction(const CodeActionInputs &Inputs,
+                  Callback<std::vector<CodeActionFix>> CB);
+
----------------
maybe a more structured output can be useful here:
```
struct CodeActionResult {
  std::string Version; // Version of content results belong to.
  struct QuickFix {
      DiagRef D; // Diagnostic being addressed by the fix.
      Fix F;
  };
  std::vector<QuickFix> QuickFixes;
  std::vector<TweakRef> TweakRefs;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155173