[PATCH] D118976: clangd: Set a diagnostic on a code action resulting from a tweak

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1011
         // We never consider refactorings etc as preferred.
         CodeAction *OnlyFix = nullptr;
         for (auto &Action : Actions) {
----------------
sammccall wrote:
> This loop is closely related to what you're doing (and the one above isn't).
> 
> Maybe we can refactor/combine as something like:
> 
> ```
> Optional<CodeAction *> OnlyFix; // nullptr means multiple fixes
> // ... loop over actions and set OnlyFix ...
> if (OnlyFix && *OnlyFix) {
>   (*OnlyFix)->isPreferred = true;
>   if (Diags.size() == 1 && Diags.front().range == Selection)
>     (*OnlyFix)->diagnostics = {Diags.front()];
> }
> ```
> 
> Note this adds the conditions:
>  - should be only one diagnostic in context
>  - should be only one quickfix action
> 
> I think these reduce the chance of false positives.
Done. Though it seems the code would work just as well with OnlyFix staying a normal pointer. Did I misunderstand something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976