
On Fri, 23 Mar 2007 10:00:43 -0400, "julien robert" <julienrob@...400...> wrote:
I began its implementation, but as i'm not sure to do it in the right way, i would like to discuss with someone having a better vision of the source code than mine.
I attach you what i've done for the moment, waiting some feedback before to continue.
Your approach looks sensible. Just a few things:
1. I don't care for the _0 suffix on the sp_document*_done functions. As C++ permits overloaded functions, just call them the same thing as their four-argument counterparts (or better, simply give the existing four-argument version an additional optional argument)
2. Rather than adding to Inkscape::Event, create a separate Inkscape::Action class which concerns itself with reperforming the action. The only thing added to Inkscape::Event would then be an optional pointer to an Inkscape::Action instance.
3. I'd suggest putting the various Inkscape::Action subclasses in an "Actions" namespace, e.g.: Inkscape::Actions::Rotate90Cw
4. Add an Inkscape::Action subclass (e.g. Inkscape::Actions::GenericAction unless you prefer another name) whose constructor receives a verb id, and whose repeat method simply invokes the verb. This should cover most of the existing verbs which don't require special parameters.
5. Make the action argument to sp_document*_done mandatory (using GenericAction with the verb id where no more specific action is applicable)
6. Move the type/verb id and description fields from Inkscape::Event to Inkscape::Action, and remove those arguments from sp_document*_done, since they'd then be supplied by the action
Thanks for starting on this work; it will serve as the basis not only for the repeat feature, but also many other important things too.
-mental