sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a project: clang-tools-extra.
Should we have a test somewhere that tweaks defined in modules actually work?
================
Comment at: clang-tools-extra/clangd/FeatureModule.h:97
+ virtual void
+ contributeTweaks(std::vector<std::unique_ptr<class Tweak>> &Out) {}
+
----------------
nit: can you move the forward declaration up the top? Makes the (sort-of) dep
clearer, and is less fragile re scopes when moving code around.
================
Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:55
+ if (!Modules)
+ return All;
+ for (auto &M : *Modules)
----------------
nit: the early return seems more confusing to me here than conditionally
adding...
================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
std::vector<std::unique_ptr<Tweak>>
-prepareTweaks(const Tweak::Selection &S,
+prepareTweaks(const class FeatureModuleSet *Modules, const Tweak::Selection &S,
llvm::function_ref<bool(const Tweak &)> Filter);
----------------
forward decl: and here
================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
std::vector<std::unique_ptr<Tweak>>
-prepareTweaks(const Tweak::Selection &S,
+prepareTweaks(const class FeatureModuleSet *Modules, const Tweak::Selection &S,
llvm::function_ref<bool(const Tweak &)> Filter);
----------------
sammccall wrote:
> forward decl: and here
nit: move the modules to the end of the list?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98498/new/
https://reviews.llvm.org/D98498
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
