sammccall added a comment.
Thanks, this looks a bunch simpler.
I have a question about the direction here: AFAICS there's various efforts in
the refactoring infrastructure to allow consumers of the Refactor libs (e.g.
clang-refactor, clangd, XCode?) to operate without knowing the details of the
refactorings.
One example is the treating the identifying `Name` of a rule as data, instead
of referring to a subclass directly. Another is the way options are
communicated via `OptionsVisitor` and `Requirement`s etc, where they could be
more directly expressed as members of a `Rule`-specific struct or parameters to
a function.
An interface that truly decouples N refactorings from M consumers has
scalability advantages, but there are some things that cut against it:
- It tends to add complexity/indirection, which can slow down contributors,
mask bugs, and makes certain features (ones that don't fit into the existing
framework) hard to add.
- If the generic interfaces aren't enough we pierce them, resulting in both
coupling *and* complexity. e.g. Clangd really needs control over how
refactorings are exposed: which ones are visible under what names, how the
JSON-RPC messages are structured. (Because its protocol isn't really an
implementation detail under our control, we need to be able to adapt
to/influence editors and evolution of the LSP standard).
What's the biggest value we get out of the generic interface? Whose life would
get harder if refactorings had this strawman interface/concept?
template<typename Options, typename Result>
class Refactoring {
bool available(RefactoringContext&, const Options&) = 0;
Error invoke(RefactoringContext&, const Options&, RefactoringConsumer&) =
0;
}
@klimek may have thoughts here.
================
Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:60
+ /// associated with this rule.
+ virtual Optional<std::pair<StringRef, StringRef>> getEditorCommandInfo() {
+ return None;
----------------
ioeric wrote:
> I think `getEditorCommandInfo` might be a wrong name here.
>
> IMO, all end rules (i.e. editor-facing rules) should have name information.
> It might make sense to introduce a structure that holds all metadata about a
> rule as well as an interface that returns such a structure. With that, we
> also don't need to update the API when more rule information is added in the
> future.
>
> I also think the interface should be pure virtual, and all end rules should
> implement this interface since they should have names or metadata of some
> sort.
I like the idea of to moving metadata like Title to the RefactoringActionRule
through a method like getEditorCommandInfo(), though as Eric says a struct
would be clearer.
Is there any reason to make it optional and rebindable? Surely we can come up
with a reasonable name for each rule, and if the editor wants to use a
different name for some purpose, it can do so without help from the framework
(e.g. put the RefactoringActionRule in a struct).
Repository:
rL LLVM
https://reviews.llvm.org/D38985
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
