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:
> arphaman wrote:
> > 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.
> Currently, we have `createRefactoringActions` as the API of the refactoring
> engine/library. I think we could introduce a `RefactoringEngine` class that
> exposes all user-facing interfaces from the engine, including
> creating/accessing actions and providing the mapping of editor command. And
> the command registration could be handled inside the class, which I assume
> would be simpler. This would also allow us to expose more interfaces in the
> future via this class.
>
> (Sorry, I didn't make it to the dev meeting this year... I hope I could get a
> chance to meet you in the next dev meeting or the European dev meeting early
> next year :)
I think that's a good idea.
I will post a follow up patch that moves the rule creation to the engine. This
one still creates the editor binding in the file.
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
