arphaman added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:192
+ Intent intent() const override { return Intent::Refactor; }
+ std::string title() const override { return "Inline function definition"; }
+
----------------
sammccall wrote:
> kadircet wrote:
> > arphaman wrote:
> > > Sorry, I'm not really familiar with design of tweaks, so this might be a
> > > bad question: Is it possible to change the title of the tweak on a per-TU
> > > basis. More specifically, some tweaks might want to have different titles
> > > for Objective-C actions compared to their C++ siblings.
> > Current design returns a constant string per tweak, but we can change that
> > behaviour easily by making this a function of language options. You can
> > take a look at `ClangdServer::enumerateTweaks` for details.
> This isn't quite true: id() must be constant but title() need not be. See
> ExpandMacro for an example.
>
> The only constraint is it shouldn't be terribly expensive (this is why
> ExpandAuto doesn't include the type in the title, due to an AST bug it
> requires a new traversal).
>
> > More specifically, some tweaks might want to have different titles for
> > Objective-C actions compared to their C++ siblings.
> I think this is possible and the right idea. But for completeness I should
> mention: having multiple Tweak subclasses that share implementation is also
> possible. In particular in cases where you have two related actions and may
> want to provide *both* as options (maybe extract function vs method?), they'd
> need to be separate subclasses. (Or we'd need to change the design a bit)
Oh yes, I think it's probably better to use the `Tweak` subclasses like you
said. This will be better for ObjC++, which could have either a C++ or an ObjC
tweak available depending on the current AST selection.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65433/new/
https://reviews.llvm.org/D65433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
