QAbstractItemModel with QUndoStack

I have a custom QAbstractTableModel descendant that I want to extend with a QUndoStack to allow undo/redo of changes to the model.

How would I do this nicely? I have two ideas but I am unhappy with both.

I created a simple QUndoCommand descendant:

Code:

---

class SetDataCommand: public QUndoCommand
{
public:
	SetDataCommand(const QPersistentModelIndex & index, const QVariant & oldValue, const QVariant & newValue)
		: m_index(index),
		  m_oldValue(oldValue),
		  m_newValue(newValue)
	{}
 
	void undo();
	void redo();
private:
	const QPersistentModelIndex m_index; ///< Holds the model index.
	const QVariant m_oldValue; ///< Holds the old value.
	const QVariant m_newValue; ///< Holds the new value.
};
 
void SetDataCommand::undo()
{
	if (m_index.isValid())
		const_cast<QAbstractItemModel *>(m_index.model())->setData(m_index, m_oldValue);
}
 
void SetDataCommand::redo()
{
	if (m_index.isValid())
		const_cast<QAbstractItemModel *>(m_index.model())->setData(m_index, m_newValue);
}

---

Idea 1:
The setData(...) method changes the model data and adds the undo command to the undo stack. Problem: Pushing the undo command to the stack will execute the redo() method of the command.
Workaround: The redo command has a boolean guard that will lead the first call to redo() do nothing. Both undo() and redo() would use a user role to make changes to the model to prevent the creation of undo commands when using undo/redo.

Idea 2:
The setData() method creates the undo command which will then lead to the call to redo() that will call some protected? custom method that will modify the model. The problem is that I would not know in the setData() method, if the undo command succeded with redo() and would return true without knowing.
Workaround: I would have to do all checks before creating the undo command to be certain that the undo command will not fail to modify the data.

Thanks in advance for comments and ideas,
-Jens

Hmm... I wouldn't do that to a model :) The model should just be an interface between the application and the data (it shouldn't contain any "logic"). The model can change rapidly, causing the undo stack to be very large and it can be hard to sync the stack with the model if the model gets regenerated or something like that (I don't know exactly what you want to keep in the model).

You might also encounter problems when doing redo/undo - each command should cause a dataChanged signal to be emitted for both redo and undo. If the commands were to change the model, they'd need to have access to its internal structures (to modify the data) and to the model itself (to emit the signal).

I would suggest to go the other way round - have a QUndoStack that is using QUndoCommands which modify the model using the model interface (setData). You could use the stack for high-level "object oriented" access to your application's functionality and the stack would then call the "low-level" model interface for you. The only problem I see is modifying the model behind the stack's back - you'd have to disallow that.

Quote:

---

Originally Posted by wysota

Hmm... I wouldn't do that to a model :) The model should just be an interface between the application and the data (it shouldn't contain any "logic"). The model can change rapidly, causing the undo stack to be very large and it can be hard to sync the stack with the model if the model gets regenerated or something like that (I don't know exactly what you want to keep in the model).

---

I thought to clear the undo stack when the underlying datastructure is exchanged. All other changes to the datastructure would be done using the model interface.

Quote:

---

Originally Posted by wysota

You might also encounter problems when doing redo/undo - each command should cause a dataChanged signal to be emitted for both redo and undo. If the commands were to change the model, they'd need to have access to its internal structures (to modify the data) and to the model itself (to emit the signal).

---

The undo / redo commands would use the standard model interface or some added custom interface so that the model would emit dataChanged and make use of beginInsert... etc.

Quote:

---

Originally Posted by wysota

I would suggest to go the other way round - have a QUndoStack that is using QUndoCommands which modify the model using the model interface (setData). You could use the stack for high-level "object oriented" access to your application's functionality and the stack would then call the "low-level" model interface for you. The only problem I see is modifying the model behind the stack's back - you'd have to disallow that.

---

The problem for me is how to implement this. I have a filtered table view on the model and a configurable number of widgets using a QDataWidgetMapper.
I came to my ideas because I would get the information to change data in setData, insertRows and deleteRows. I would only have to implement these and some custom interface methods to insert (or paste) / cut whole row selections and have them push undo commands to my undo stack.

For user commands like "insert table row (before/after)" "delete table row" I could implement your idea easyly. But how would I add changes done using the table view and the data widget mapper. (In my table only rows can be selected and the current "active" row data is shown partly in the table and the rest in the mapped widgets)

Thanks in advance,
-Jens

Quote:

---

Originally Posted by No-Nonsense

The undo / redo commands would use the standard model interface or some added custom interface so that the model would emit dataChanged and make use of beginInsert... etc.

---

Ok, but if you use setData() in your app that creates a QUndoCommand which calls setData again, you'll have an endless recursion. Use of custom methods is a solution here of course.

Quote:

---

The problem for me is how to implement this. I have a filtered table view on the model and a configurable number of widgets using a QDataWidgetMapper.

---

Well, in that case it'd be hard to do it this way.

Quote:

---

Originally Posted by wysota

Ok, but if you use setData() in your app that creates a QUndoCommand which calls setData again, you'll have an endless recursion. Use of custom methods is a solution here of course.

---

This is wy I thought of either the custom methods or to use a custom role for undo/redo. E.g. the model is modified from the outside using setData(..., Qt::EditRole) and undo/redo use setData(..., UndoRedoRole) so i would not create a undo command when accessed using the UndoRedoRole.

But then my problem remains: Would I prevent the first call to redo (i.e. have it do nothing) to be able to return false from setData or would I return true, even if nothing was changed (because the created undo command failed changing the data in redo).

-Jens

If changing the model is a cheap operation, I would ignore the situation and let the model set the data to what it was before.

BTW. Monitoring the EditRole might not be enough - depends on the model and how versatile you want your solution to be.

hi,
following the same issue i thought it might be feasible to use a proxy model for that. this, of course, would offer reusability and reduce the complexity of the actual model greatly.

now, 2 problems come to my mind:
1) signal connection. when undoing, the source model's events shouldn't be added to the undo stack. a simple, but somewhat hacky approach could be to just disconnect from the source model while undoing.
2) undoing insert/remove item events. i guess the structure of the model had to be parsed, eg. the cols/rows upwards from a removed item, to insert it back again at this very positions. seems also a little hacky...

am i missing some points that could turn out to be problematic?

You could add to your model private section friend class SetDataCommand. Then can call protected function of model in the SetDataCommand.