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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1011
         // We never consider refactorings etc as preferred.
         CodeAction *OnlyFix = nullptr;
         for (auto &Action : Actions) {
----------------
ckandeler wrote:
> 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?
Oops, I missed the `break` in the loop.
Yes a normal pointer would be better, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118976