On Fri, Feb 25, 2011 at 1:43 PM, Andy Allan <gravityst...@gmail.com> wrote: > On Fri, Feb 25, 2011 at 1:17 PM, Steve Bennett <stevag...@gmail.com> wrote: > >> Does this absolutely have to be the case? I can't quite understand, >> from a theoretical point of view, why this principle is necessary. Why >> not add the action to a stack, and also carry it out now? What's the >> benefit of maintaining the current state as long as possible, then >> doing all the actions in one flurry? > > This is where I could be wrong, but I think that it's critical to the > redo part. When undo'ing an action the action is added to the redo > list, so if adding an action has side-effects that'll blow up during > redoing the action. Also, if the action works based on side effects > that aren't tracked within the action itself, then it won't work > properly when being redone. Hence everything needs to be done by > adding things to an action, and it needs to be completely > self-contained so it can be moved back and forward between the > different stacks. > > But I might be wrong.
You're not. That's exactly why you need to do it that way -- if you're doing a simple composite undoable action anyway. It may get a little confusing, but you can of course create your own UndoableAction class. In that case during the doAction method you're free to make any direct changes you like, calling other action's doAction methods to do so. The only catch is that you have to be able to put things back again how you found them (probably by calling undoAction in the right order). And then properly handle a redo. The easiest method of doing that is override CompositeUndoableAction, pushing actions as you do them, allowing the undo be handled normally, except that it'd have to clear the action list when done. It generally cleaner and safer to use the existing actions and compose. If you don't understand what I just said then I suggest sticking to the existing actions as there's less scope for breaking things! > >> And just to add a bit more incoherent rambling, would a simpler model >> be, instead of passing around these stacks everywhere, to have just a >> main undo stack with two functions: >> 1) RecordAction which adds an action to the undo stack, and carries it out. >> 2) FinishUndoBlock, which groups all actions since the last time it >> was called, into one block, which will all be undone with a single >> keystroke. Sort of the equivalent of closing a changeset. > > That's just an unclear representation of a composite action! The > RecordAction is undo.push, the FinishUndoBlock is > MainUndoStack....addAction(), and the > carrying-out-before-we're-finished is the bad idea that we need to > avoid. > Yeah, what we have is a nested and OO implementation of a simple stack with markers. This allows you to do cool stuff like encapsulate complex actions without bothering the user with extra undo steps or adding lots of boiler plate code around the "RecordAction" stage. Dave _______________________________________________ Potlatch-dev mailing list Potlatch-dev@openstreetmap.org http://lists.openstreetmap.org/listinfo/potlatch-dev
