Using QUndoStack and QUndoCommand with a QTableView

[KlodKrichen]

I am using a QTableView which has a model that inherits from QAbstractTableModel. I added a QUndoStack to my application and I am trying to push into the stack a QUndoCommand whenever the user edit the table entries which will later implicitly call setdata() method of the model.
This is my command class:

EditCellCommand::EditCellCommand(QAbstractTableModel *model, const QModelIndex &index, const QVariant &oldvalue, const QVariant &newvalue,QUndoCommand *parent)
    : QUndoCommand(parent), MyModel(model),myIndex(index)
    , oldData(oldvalue), newData(newvalue)
{
}
void EditCellCommand::undo()
{
    MyModel->setData(myIndex,oldData,Qt::EditRole);
}

void EditCellCommand::redo()
{
    MyModel->setData(myIndex,newData,Qt::EditRole);
}

The problem is that pushing a command to the stack will always call redo() and therefor the setdata() will be executed two times the first one implicitly when the user changes the table data, and the second when pushing command to the stack.

I think that I am missing something here, is there a solution to prevent this type of behavior?

[JonB]

@KlodKrichen said in Using QUndoStack and QUndoCommand with a QTableView:

The problem is that pushing a command to the stack will always call redo() and therefor the setdata() will be executed two times

Yep, this is a "feature" of the way they made QUndoStack work. And while I get the logic I have symapthy with you & others that this is not always "convenient"!

You have only two basic choices:

• Make it so setData() is not called directly outside of redo(). However you achieve that, the outside world has to go through your redo().

• Keep a bool member variable in your EditCellCommand class. Use that to test whether redo() is being called "first time" (in your case you want it to do nothing then, because the outside world has just done the setData()) or "subsequent time" (you know you do want it do its setData() now, because it's a genuine undo-redo). A bit ugly, but works.

[KlodKrichen]

Thanks for the reply I will try what you suggested.

[KlodKrichen]

@JonB The first problem is solved by following the second approach that you described, now the second challenge will be the right place to push the undocommand into the stack by calling undoStack->push(cmd). I thought about connecting a slot to the dataChanged signal and pushing the command from that slot but the problem is that the cell data already edited and I have access only to the current data from the model. I tried pushing the command to the stack directly from setdata() where I have access to the old data before updating the model but that was a big fail from my side since it created a lot of problems. Is there a way to retrieve the previous data from the model ?

[JonB]

@KlodKrichen
I think undo/redo() will have to call setData(), not the other way round. The world should go through the undo level if it wants undoable actions. Not via setData() or dataChanged() (or at least that gets tricky). So undoStack->push(cmd) becomes the level at which the outside world makes an update. You can fetch the current data to save in the command just before you setData() to change it.

[KlodKrichen]

Exactly, and finding that specific place just before setData() change the cell data seems to be tricky since setData() is called implicitly while working with QtableView and QAbstractTableModel. One way around that would be to subclass QStyledItemDelegate and assigning the delegate to the table columns with setItemDelegate so that all the cell editing will go through the Delegate with setModelData() where we can call undoStack->push(cmd) just before calling explicitly setData() and we have access to both the new and old data.

[JonB]

@KlodKrichen said in Using QUndoStack and QUndoCommand with a QTableView:

since setData() is called implicitly while working with QtableView and QAbstractTableMode

When are you saying this is the case? setData() should only be called from your own code explicitly or from the user-edit. In both cases you should push to the undo stack rather than calling setData() directly.

One way around that would be to subclass QStyledItemDelegate and assigning the delegate to the table columns with setItemDelegate so that all the cell editing will go through the Delegate with setModelData() where we can call undoStack->push(cmd) just before calling explicitly setData() and we have access to both the new and old data.

That's exactly what you must do for user editing.