sammccall added a comment. Offline discussion:
- CRTP doesn't buy anything. Simple inheritance is less clean than current function-passing version but probably more familiar. - `Tweak` is a good name: short, evocative but not commonly used, works as a noun and a verb. - Registry might be a good enough fit to use directly Here's the result of my local, er, tweaking to `ActionProvider.h`. //===--- Tweak.h -------------------------------------------------*- C++-*-===// // // The LLVM Compiler Infrastructure // // This file is distributed under the University of Illinois Open Source // License. See LICENSE.TXT for details. // //===----------------------------------------------------------------------===// // A tweak is a small context-sensitive refactoring-like action that is // triggered at a cursor location, examines the AST and produces a set of edits. // // It has two main pieces of logic: // - prepare() quickly checks whether the action is applicable at the cursor. // It must be cheap as all tweaks are prepared to list them for the user. // - apply() computes the actual edits. // This can be more expensive, only the chosen tweak is applied. //===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_ACTIONPROVIDER_H #include "ClangdUnit.h" #include "Protocol.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" namespace clang { namespace clangd { // Abstract base for small context-sensitive refactoring actions. class Tweak { public: // Information about the location where the tweak was triggered. struct Selection { // The path of an active document the action was invoked in. llvm::StringRef File; // The text of the active document. llvm::StringRef Code; // Parsed AST of the active file. ParsedAST &AST; // A location of the cursor in the editor. SourceLocation Cursor; // FIXME: full selection bounds. // FIXME: AST node under cursor. // FIXME: allow tweaks to specify matchers, and include match results. // FIXME: allow access to index and other source files. }; // Tweaks must have a no-op default constructor. virtual ~Tweak() = default; virtual const char* id() = 0; // Determines whether this tweak can run at the selection. // This function may record information to be used later. // It should run as fast as possible, particularly when returning false. virtual bool prepare(const Selection &) = 0; // Generate the edits for this tweak. // REQUIRES: prepare() was called and returned true. virtual llvm::Expected<tooling::Replacements> apply(const Selection &) = 0; // Description of this tweak at the selected location. // e.g. "Out-line definition of toString()". // REQUIRES: prepare() was called and returned true. virtual std::string describe() const; }; // All tweaks must be registered, next to their definition. #define REGISTER_TWEAK(Subclass) \ static ::llvm::Registry<::clang::clangd::Tweak>::Add<Subclass> \ TweakRegistrationFor##Subclass(Subclass{}.id(), #Subclass); // Calls prepare() on all tweaks, returning those that can run on the selection. std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &); // Calls prepare() on the tweak with a given ID. // If prepare() returns false, returns an error. Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef ID, const Tweak::Selection &); } // namespace clangd } // namespace clang #endif Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56267/new/ https://reviews.llvm.org/D56267 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits