"Repeat last action feature", need some advise
A feature requested by spikevision is the following:
Repeating the last action
No "Repeating" is different than "redoing". It refers to applying the same action again on an object or on a different one. Redoing reapplies the action that was undone. Some drawing programs, like CorelDraw, give the ability to repeat the last action(CTRL-R). For exemple, if you applied a color on an object, you could select another object and repeat the fill action. The second object will have the same color. If you performed a "rote/move/leave original" action on an object, this action can be applied to the newly created object. This can be handy when creating patterns like in the european flag exemple of "A Guide to Inkscape" by Tavmjong Bah. This would be faster since it could be done with just a few keyboard shortcuts.
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.
Also, spikevision sent me some icons that he drew for the 'repeat last action' button, and i forward it to you : which one do you like more ? (to me, it's number 6)
Cordially,
cbvJohn
On 3/23/07, 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 see you are mostly working with undo code there. This approach is certainly more general that "repeat the last verb" because undo records all actions, while verbs exist for only a subset of all actions. However, here lies a problem: undo log only records the (possibly labelled and keyed) changes in the XML tree and nothing else. It does not remember which object the action was applied to, and thus it cannot reapply the same action to a different object. And without this, this functionality will be of very limited use. How are you planning to attack this?
Also, from a cursory look at your patch, it's quite big and seems to have stuff that does not belong, such as differences in clone transforms. This is likely because the SVN head has diverged during the time you were working on the patch. You need to merge all the latest changes into your working copy before diffing, otherwise your patch may inadvertently revert some of the changes in SVN.
On 3/23/07, bulia byak <buliabyak@...400...> wrote:
On 3/23/07, 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 see you are mostly working with undo code there. This approach is certainly more general that "repeat the last verb" because undo records all actions, while verbs exist for only a subset of all actions. However, here lies a problem: undo log only records the (possibly labelled and keyed) changes in the XML tree and nothing else. It does not remember which object the action was applied to, and thus it cannot reapply the same action to a different object. And without this, this functionality will be of very limited use. How are you planning to attack this?
I'm not sure to follow you... actually, i'm not using undo log for repeating and i'm just recording every event when it is submitted in the 'sp_document_done' function, so that i can remember more or less everything, and reapply the action to different objects (this is done by defining different Event objects -in event.h-, each corresponding to a different action, and storing the necessary variables to be able to call the corresponding function with the good arguments when repeating on a new object/selection). Does this make sense ?
Also, from a cursory look at your patch, it's quite big and seems to
have stuff that does not belong, such as differences in clone transforms. This is likely because the SVN head has diverged during the time you were working on the patch. You need to merge all the latest changes into your working copy before diffing, otherwise your patch may inadvertently revert some of the changes in SVN.
Really sorry about that, i forgot to do it. Btw How can i keep the modification on other things so that it wont appear in the patch without deleting it in my working copy (i want to take a look at it later..) ? Do you want the patch without this disturbing part (which actually represent few lines of this -quite big- patch)?
On Fri, 23 Mar 2007 14:51:13 -0400, "julien robert" <julienrob@...400...> wrote:
Really sorry about that, i forgot to do it. Btw How can i keep the modification on other things so that it wont appear in the patch without deleting it in my working copy (i want to take a look at it later..) ?
With SVN, it's usually best to maintain separate working copies for separate sets of changes.
-mental
On Fri, 23 Mar 2007 13:33:47 -0700, MenTaLguY <mental@...3...> wrote:
With SVN, it's usually best to maintain separate working copies for separate sets of changes.
Incidentally, with SVN it's also best not to let your working copy diverge from trunk too much. We should look for intermediate stable points where we can commit your work so far to trunk, so we don't have a huge pile of integration to do at the very end.
-mental
On Fri, 23 Mar 2007 14:18:16 -0400, "bulia byak" <buliabyak@...400...> wrote:
It does not remember which object the action was applied to, and thus it cannot reapply the same action to a different object. And without this, this functionality will be of very limited use. How are you planning to attack this?
I think annotations in the undo log is the right place to attack this -- if needed, the affected and any other relevent information can be captured in the Event (now Action) object as required.
How do you (bulia) feel about having undo/redo save/restore the selection, by the way? That may make matters even easier.
-mental
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
participants (3)
-
bulia byak -
julien robert -
MenTaLguY