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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
