arphaman added inline comments.
================
Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:58
+ /// Returns the editor command that's was bound to this rule.
+ virtual const EditorCommand *getEditorCommand() { return nullptr; }
};
----------------
ioeric wrote:
> I'm still not quite sure about the intention of `EditorCommand` (is it
> supposed to be used as a mapping from name to rule, or the other way
> around?), but I'm a bit concerned about mixing editor logic with the
> refactoring rules this way. Also to enable a editor command, it seems to me
> that we would need to touch both the registry and the rule creation code,
> which seems a bit cumbersome.
>
> I think we should either 1) separate out the command logic cleanly without
> touching action/rule interfaces in the refactoring engine or 2) simply bake
> the logic into the refactoring engine.
>
> It is unclear to me if 1) is possible, but for 2), I think we could introduce
> a `RefactoringEngine` class which carries all refactoring actions as well as
> a map to serve the purpose of `EditorCommand`. And I think by doing this, we
> could also make the interfaces of `RefactoringEngine` more explicit.
(Quick comment before the devmeeting:)
Mapping from name to rule.
You're right though, It might be better to explore an alternative solution, but
I don't quite follow your proposal yet. I'll be in the tooling hacker's lab at
the dev meeting today if you want to discuss this in person.
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
