Cancel to QUndoStack in redo()

[iwatsu]

Could somebody confirm that it's not possible to abort the push to the stack if something happens in the redo() function? I only found one related post that didn't have an answer.

Thanks in advance!

[Rachol]

Well you could always do some "hack" stuff. Get the pointer to the undoStack object in the constructor of your command. Use that object to call undo() and revert the stuff that failed in you redo(). This is the same command object, so it should be quite easy to do with some flags to handle it properly.

[wysota]

Why do you want to cancel a redo()?

[iwatsu]

Why do you want to cancel a redo()?

The operation that I'm doing in redo might fail in rare cases. If that happens I don't want to have a QUndoCommand that doesn't do anything (since the actual operation was never performed)

[wysota]

Maybe you can check the condition before before you call redo?

[iwatsu]

yes, I might just have to refactor some code or try Rachol's hack.

Thanks

[tryptik]

This is a great topic. The QUndoCommand is a fine implementation of the command pattern, except for the fact that you can't roll back an incomplete redo operation. I have had to customize the redo to 'skipInitialRedo' - so I can add an undo command to the stack after the operation has completed successfully, and to prevent the stack from re-redoing when items are pushed onto it, at which point I've basically implemented my own command pattern and now have to integrate it with the undocommand.

It would be awesome to not have to always hack the damn undo command - if I could just catch an exception around the undostack.push and then pop off a bad command, I think the undo system would be much more useful. This wouldn't be too hard to add, and would make the UndoCommand useful.

[bootchk]

Why not subclass QUndoCommand, for example TransactionalUndoCommand. You don't want to hide everything in your program. It should be explicit that some commands (instances of TransactionalUndoCommand) can be rolled back or committed. Programming using exceptions is not always a good idea: a rolled back command may not be exceptional, but normal.

For example, in my use, a TransactionalUndoCommand is used to ghost a dragged something. The command is executed, and the results are visible, but if the user's drag turns out to be small, it gets rolled back, meaning that the user changed their mind (but doesn't have to explicitly undo), else it is committed(), which pushes it on the command stack (but redo() doesn't execute it on this initial redo.)

On the other hand, the Qt documentation should say what happens if the redo() that happens on a call to push() throws an exception. In other words, during push() does the command first get put on the stack, then redo() called on it, or vice versa? It must be the former, otherwise this thread wouldn't exist.

[tryptik]

Why not subclass QUndoCommand, for example TransactionalUndoCommand. You don't want to hide everything in your program. It should be explicit that some commands (instances of TransactionalUndoCommand) can be rolled back or committed. Programming using exceptions is not always a good idea: a rolled back command may not be exceptional, but normal.

But then there are actual exceptional circumstances. I'm not the genius I'd like to be, so encountering an exception happens in complicated code and whether or not your command causes an exception or manages error codes, you can still end up with an ineffective command on your stack and no way to pop it off, unless you write your own command pattern.

I could attribute it to laziness, but an exceptional design should encourage laziness - the lazy course is also the most efficient, and the most efficient course should also be the best practices of a library.

For example, in my use, a TransactionalUndoCommand is used to ghost a dragged something. The command is executed, and the results are visible, but if the user's drag turns out to be small, it gets rolled back, meaning that the user changed their mind (but doesn't have to explicitly undo), else it is committed(), which pushes it on the command stack (but redo() doesn't execute it on this initial redo.)

I've done this before as well, and it is fine. What I find coming up in my work is a pattern where a simple derivation of a QUndoCommand would do the job, without any functors or custom logic for the when's and how's of the execution. I was using an undoCommand class that was a shell holding a list of functors, so I could drop it on the stack the post-populate it, but, again, that's a lot of ground work that needs to be done even for simple undo in a simple tool - I write a lot of simpler tools in PyQt.

My latest trick is using a macro on the undo stack, dropping a bunch of stuff in there and rolling back if there is a fail. I still can't pop the stack, but I very much appreciate the simplicity in my code.

It may come down to a simple matter of taste, but the undo system seems to be at 80% of where it could be, and I think that extra nudge would make it a lot more useful. If I get some time I may crack open the open-source code, add it, and see if it blows up in my face.

[regular]

Sorry to reply to such an old thread but I didn't want to create a new one.

I have a very similar problem. I have a QGraphicsScene and all the operations on it are done using undo commands. I have a place in the application to import from various sources. I want the import to be a single undoable operation. So what I did was to surround the code that does the import in a QUndoStack::endMacro() and QUndoStack::beginMacro(). However, the user can choose to cancel the process in the middle in some cases (depending on where you import from) - in such cases I end up with an empty undo command. Here's some code to illustrate the situation:

IImporter *importer;
...
// set the importer to be a concrete instance
...
undoStack->beginMacro("Import");
...
if(!importer->importItems()) // a complex operation, many things could happen inside or none at all depending on the importer
{ // the operation didn't import anything or the user canceled
   ...
   undoStack->endMacro(); // I know this failed but I can't remove the empty added command from the stack
   return;
}
...
undoStack->endMacro();

IImporter *importer;
...
// set the importer to be a concrete instance
...
undoStack->beginMacro("Import");
...
if(!importer->importItems()) // a complex operation, many things could happen inside or none at all depending on the importer
{ // the operation didn't import anything or the user canceled
   ...
   undoStack->endMacro(); // I know this failed but I can't remove the empty added command from the stack
   return;
}
...
undoStack->endMacro();

To copy to clipboard, switch view to plain text mode 

I could bring the macro creation in each importer but that seems like a bad idea maintenance-wise (there are a lot of importers). Any suggestions? I agree that there should be a way to pop a command from the undo stack.

[wysota]

Import first (the user can cancel here), create the command later passing it the imported data structure, activate the structure in redo(), deactivate it in undo(). You don't want to "reimport" data each time user does a redo, right?

[regular]

Hmm OK, that makes sense. I'll have to use a modified version of your advice.

Just to clarify, what is undone and redone right now is not the whole "import" process (i.e. parsing/interpreting the source) but just the operations on the scene (add, remove, set position, resize, etc.) since only those are implemented as undo commands. Because all of those commands are embedded in the scene itself and the importers work on it without knowledge of the undo commands, they have no control. Thankfully though, the scene itself can be set to not use undo actions internally so I'll use that before an import and follow your advice from there.

Thanks