How to undo edit of QStandardItem in PySide/PyQt?

Question

Using the identical question asked about QListWidgets as a guide, I am trying to make a QStandardItemModel in which I can undo the edit of an item.

As can be seen in the SSCCE below, I'm pretty much copying the example exactly, but with some minor tweaking because currentItemChanged isn't available for QStandardItemModel. To get around that, I'm using the clicked signal instead to fix the previous text for an item.

Strangely, the correct description shows up in the undostack, but when I click the undo button it doesn't actually undo anything.

Note the present question is superficially the same as this question. The answer that was accepted at that other version is less an answer, more a hint. It's a hint I am trying to implement here, but it is not working yet. Since this question is more specific and detailed, it shouldn't count as a duplicate, IMO.

SSCCE

from PySide import QtGui, QtCore
import sys

class CommandItemEdit(QtGui.QUndoCommand):
    def __init__(self, model, item, textBeforeEdit, description = "Item edited"):
        QtGui.QUndoCommand.__init__(self, description)
        self.model = model
        self.item = item
        self.textBeforeEdit = textBeforeEdit
        self.textAfterEdit = item.text()

    def redo(self):
        self.model.blockSignals(True)  
        self.item.setText(self.textAfterEdit)
        self.model.blockSignals(False)

    def undo(self):
        self.model.blockSignals(True)
        self.item.setText(self.textBeforeEdit)
        self.model.blockSignals(False)     


class UndoableTree(QtGui.QWidget):
    def __init__(self, parent = None):
        QtGui.QWidget.__init__(self, parent = None)
        self.setAttribute(QtCore.Qt.WA_DeleteOnClose)
        self.view = QtGui.QTreeView()
        self.model = self.createModel()
        self.view.setModel(self.model)
        self.view.expandAll()
        self.undoStack = QtGui.QUndoStack(self)
        undoView = QtGui.QUndoView(self.undoStack)
        buttonLayout = self.buttonSetup()
        mainLayout = QtGui.QHBoxLayout(self)
        mainLayout.addWidget(undoView)
        mainLayout.addWidget(self.view)
        mainLayout.addLayout(buttonLayout)
        self.setLayout(mainLayout)
        self.makeConnections()
        #For undo/redo editing
        self.textBeforeEdit = ""

    def makeConnections(self):
        self.view.clicked.connect(self.itemClicked)
        self.model.itemChanged.connect(self.itemChanged)
        self.quitButton.clicked.connect(self.close)
        self.undoButton.clicked.connect(self.undoStack.undo)
        self.redoButton.clicked.connect(self.undoStack.redo)

    def itemClicked(self, index):
        item = self.model.itemFromIndex(index)
        self.textBeforeEdit = item.text()  

    def itemChanged(self, item):
        command = CommandItemEdit(self.model, item, self.textBeforeEdit, 
            "Renamed '{0}' to '{1}'".format(self.textBeforeEdit, item.text()))
        self.undoStack.push(command)


    def buttonSetup(self):
        self.undoButton = QtGui.QPushButton("Undo")
        self.redoButton = QtGui.QPushButton("Redo")
        self.quitButton = QtGui.QPushButton("Quit")
        buttonLayout = QtGui.QVBoxLayout()
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.undoButton)
        buttonLayout.addWidget(self.redoButton)
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.quitButton)
        return buttonLayout

    def createModel(self):
        model = QtGui.QStandardItemModel()
        model.setHorizontalHeaderLabels(['Titles', 'Summaries'])
        rootItem = model.invisibleRootItem()
        #First top-level row and children 
        item0 = [QtGui.QStandardItem('Title0'), QtGui.QStandardItem('Summary0')]
        item00 = [QtGui.QStandardItem('Title00'), QtGui.QStandardItem('Summary00')]
        item01 = [QtGui.QStandardItem('Title01'), QtGui.QStandardItem('Summary01')]
        rootItem.appendRow(item0)
        item0[0].appendRow(item00)
        item0[0].appendRow(item01)
        #Second top-level item and its children
        item1 = [QtGui.QStandardItem('Title1'), QtGui.QStandardItem('Summary1')]
        item10 = [QtGui.QStandardItem('Title10'), QtGui.QStandardItem('Summary10')]
        item11 = [QtGui.QStandardItem('Title11'), QtGui.QStandardItem('Summary11')]
        rootItem.appendRow(item1)
        item1[0].appendRow(item10)
        item1[0].appendRow(item11)

        return model


def main():
    app = QtGui.QApplication(sys.argv)
    newTree = UndoableTree()
    newTree.show()
    sys.exit(app.exec_())


if __name__ == "__main__":
    main()

Answer 1

The issue appears to arise because blockSignals() is preventing the treeview from being told to repaint. I think this is because the model emits a signal to the treeview when data in the model is modified, which is obviously being blocked when you call model.blockSignals(True). If you manually resize the window after clicking undo/redo (obviously only works if there is something to undo/redo), you see that the undo/redo has actually been applied, it just didn't initially show it.

To work around this, I've modified the code so that instead of blocking signals, we disconnect the relevant signal and reconnect it. This allows the model and treeview to continue to communicate correctly while the undo/redo is in progress.

See the below code

from PySide import QtGui, QtCore
import sys

class CommandItemEdit(QtGui.QUndoCommand):
    def __init__(self, connectSignals, disconnectSignals, model, item, textBeforeEdit, description = "Item edited"):
        QtGui.QUndoCommand.__init__(self, description)
        self.model = model
        self.item = item
        self.textBeforeEdit = textBeforeEdit
        self.textAfterEdit = item.text()
        self.connectSignals = connectSignals
        self.disconnectSignals = disconnectSignals

    def redo(self):
        self.disconnectSignals()
        self.item.setText(self.textAfterEdit)
        self.connectSignals()

    def undo(self):
        self.disconnectSignals()
        self.item.setText(self.textBeforeEdit)
        self.connectSignals()


class UndoableTree(QtGui.QWidget):
    def __init__(self, parent = None):
        QtGui.QWidget.__init__(self, parent = None)
        self.setAttribute(QtCore.Qt.WA_DeleteOnClose)
        self.view = QtGui.QTreeView()
        self.model = self.createModel()
        self.view.setModel(self.model)
        self.view.expandAll()
        self.undoStack = QtGui.QUndoStack(self)
        undoView = QtGui.QUndoView(self.undoStack)
        buttonLayout = self.buttonSetup()
        mainLayout = QtGui.QHBoxLayout(self)
        mainLayout.addWidget(undoView)
        mainLayout.addWidget(self.view)
        mainLayout.addLayout(buttonLayout)
        self.setLayout(mainLayout)
        self.makeConnections()
        #For undo/redo editing
        self.textBeforeEdit = ""

    def makeConnections(self):
        self.view.clicked.connect(self.itemClicked)
        self.model.itemChanged.connect(self.itemChanged)
        self.quitButton.clicked.connect(self.close)
        self.undoButton.clicked.connect(self.undoStack.undo)
        self.redoButton.clicked.connect(self.undoStack.redo)

    def disconnectSignal(self):    
        self.model.itemChanged.disconnect(self.itemChanged)

    def connectSignal(self):
        self.model.itemChanged.connect(self.itemChanged)

    def itemClicked(self, index):
        item = self.model.itemFromIndex(index)
        self.textBeforeEdit = item.text()  

    def itemChanged(self, item):
        command = CommandItemEdit(self.connectSignal, self.disconnectSignal, self.model, item, self.textBeforeEdit, 
            "Renamed '{0}' to '{1}'".format(self.textBeforeEdit, item.text()))
        self.undoStack.push(command)


    def buttonSetup(self):
        self.undoButton = QtGui.QPushButton("Undo")
        self.redoButton = QtGui.QPushButton("Redo")
        self.quitButton = QtGui.QPushButton("Quit")
        buttonLayout = QtGui.QVBoxLayout()
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.undoButton)
        buttonLayout.addWidget(self.redoButton)
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.quitButton)
        return buttonLayout

    def createModel(self):
        model = QtGui.QStandardItemModel()
        model.setHorizontalHeaderLabels(['Titles', 'Summaries'])
        rootItem = model.invisibleRootItem()
        #First top-level row and children 
        item0 = [QtGui.QStandardItem('Title0'), QtGui.QStandardItem('Summary0')]
        item00 = [QtGui.QStandardItem('Title00'), QtGui.QStandardItem('Summary00')]
        item01 = [QtGui.QStandardItem('Title01'), QtGui.QStandardItem('Summary01')]
        rootItem.appendRow(item0)
        item0[0].appendRow(item00)
        item0[0].appendRow(item01)
        #Second top-level item and its children
        item1 = [QtGui.QStandardItem('Title1'), QtGui.QStandardItem('Summary1')]
        item10 = [QtGui.QStandardItem('Title10'), QtGui.QStandardItem('Summary10')]
        item11 = [QtGui.QStandardItem('Title11'), QtGui.QStandardItem('Summary11')]
        rootItem.appendRow(item1)
        item1[0].appendRow(item10)
        item1[0].appendRow(item11)

        return model


def main():
    app = QtGui.QApplication(sys.argv)
    newTree = UndoableTree()
    newTree.show()
    sys.exit(app.exec_())


if __name__ == "__main__":
    main()

Additional Information

I've discovered that you could use you original implementation of CommandItemEdit if you explicitly call self.model.layoutChanged.emit() after unblocking the signals. This forces the treeview to update without resulting in a call to the UndoableTree.itemChanged() slot.

Note, the treeview is connected to the model signals, while the treeview is in turn connected to the UndoableTree.itemChanged() slot.

I also tried emitting the dataChanged() signal, but this ends up ultimately calling the still connected UndoableTree.itemChanged() slot, which results in infinite recursion. I think this is signal is the target of the call to model.blockSignals(), so it makes sense not to call it explicitly!

So in the end, while one of these additional methods does work, I would still go with my first answer of explicitly disconnecting the signal. This is simply because I think it is better to leave communication between the model and treeview completely intact, rather than restrict some communication while manually triggering the signals you still want. The latter route is likely to have unintended side-effects and be a pain to debug.

Answer 2

To quote another answer at a closely related question :

The clicked signal seems to be entirely the wrong way to track changes. How are you going to deal with changes made via the keyboard? And what about changes that are made programmatically? For an undo stack to work correctly, every change has to be recorded, and in exactly the same order that it was made.

The same post went on to suggest creating a custom signal that emits old/new data when the data are actually changed. Ultimately I used three ideas that I stole shamelessly from SO. First, three_pineapples' insight that disconnect is needed to avoid infinite recursion. Second, subclass QStandardItemModel to define a new itemDataChanged signal that sends both previous and new data to a slot. Third, subclass QStandardItem and have it emit this signal when the data is changed: this is handled in a reimplementation of setData()).

Here is the full code:

# -*- coding: utf-8 -*-

from PySide import QtGui, QtCore
import sys

class CommandTextEdit(QtGui.QUndoCommand):
    def __init__(self, tree, item, oldText, newText, description):
        QtGui.QUndoCommand.__init__(self, description)
        self.item = item
        self.tree = tree
        self.oldText = oldText
        self.newText = newText

    def redo(self):      
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setText(self.newText)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 

    def undo(self):
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setText(self.oldText)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 


class CommandCheckStateChange(QtGui.QUndoCommand):
    def __init__(self, tree, item, oldCheckState, newCheckState, description):
        QtGui.QUndoCommand.__init__(self, description)
        self.item = item
        self.tree = tree
        self.oldCheckState = QtCore.Qt.Unchecked if oldCheckState == 0 else QtCore.Qt.Checked
        self.newCheckState = QtCore.Qt.Checked if oldCheckState == 0 else QtCore.Qt.Unchecked

    def redo(self): #disoconnect to avoid recursive loop b/w signal-slot
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setCheckState(self.newCheckState)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 

    def undo(self):
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot)
        self.item.setCheckState(self.oldCheckState)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 


class StandardItemModel(QtGui.QStandardItemModel):
    itemDataChanged = QtCore.Signal(object, object, object, object)


class StandardItem(QtGui.QStandardItem):
    def setData(self, newValue, role=QtCore.Qt.UserRole + 1):
        if role == QtCore.Qt.EditRole:
            oldValue = self.data(role)
            QtGui.QStandardItem.setData(self, newValue, role)
            model = self.model()
            #only emit signal if newvalue is different from old
            if model is not None and oldValue != newValue:
                model.itemDataChanged.emit(self, oldValue, newValue, role)
            return True
        if role == QtCore.Qt.CheckStateRole:
            oldValue = self.data(role)
            QtGui.QStandardItem.setData(self, newValue, role)
            model = self.model()
            if model is not None and oldValue != newValue:
                model.itemDataChanged.emit(self, oldValue, newValue, role)
            return True
        QtGui.QStandardItem.setData(self, newValue, role)


class UndoableTree(QtGui.QWidget):
    def __init__(self, parent = None):
        QtGui.QWidget.__init__(self, parent = None)
        self.setAttribute(QtCore.Qt.WA_DeleteOnClose)
        self.view = QtGui.QTreeView()
        self.model = self.createModel()
        self.view.setModel(self.model)
        self.view.expandAll()
        self.undoStack = QtGui.QUndoStack(self)
        undoView = QtGui.QUndoView(self.undoStack)
        buttonLayout = self.buttonSetup()
        mainLayout = QtGui.QHBoxLayout(self)
        mainLayout.addWidget(undoView)
        mainLayout.addWidget(self.view)
        mainLayout.addLayout(buttonLayout)
        self.setLayout(mainLayout)
        self.makeConnections()

    def makeConnections(self):
        self.model.itemDataChanged.connect(self.itemDataChangedSlot)
        self.quitButton.clicked.connect(self.close)
        self.undoButton.clicked.connect(self.undoStack.undo)
        self.redoButton.clicked.connect(self.undoStack.redo)

    def itemDataChangedSlot(self, item, oldValue, newValue, role):
        if role == QtCore.Qt.EditRole:
            command = CommandTextEdit(self, item, oldValue, newValue,
                "Text changed from '{0}' to '{1}'".format(oldValue, newValue))
            self.undoStack.push(command)
            return True
        if role == QtCore.Qt.CheckStateRole:
            command = CommandCheckStateChange(self, item, oldValue, newValue, 
                "CheckState changed from '{0}' to '{1}'".format(oldValue, newValue))
            self.undoStack.push(command)
            return True

    def buttonSetup(self):
        self.undoButton = QtGui.QPushButton("Undo")
        self.redoButton = QtGui.QPushButton("Redo")
        self.quitButton = QtGui.QPushButton("Quit")
        buttonLayout = QtGui.QVBoxLayout()
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.undoButton)
        buttonLayout.addWidget(self.redoButton)
        buttonLayout.addStretch()
        buttonLayout.addWidget(self.quitButton)
        return buttonLayout

    def createModel(self):
        model = StandardItemModel()
        model.setHorizontalHeaderLabels(['Titles', 'Summaries'])
        rootItem = model.invisibleRootItem()
        item0 = [StandardItem('Title0'), StandardItem('Summary0')]
        item00 = [StandardItem('Title00'), StandardItem('Summary00')]
        item01 = [StandardItem('Title01'), StandardItem('Summary01')]
        item0[0].setCheckable(True)
        item00[0].setCheckable(True)
        item01[0].setCheckable(True)
        rootItem.appendRow(item0)
        item0[0].appendRow(item00)
        item0[0].appendRow(item01)
        return model


def main():
    app = QtGui.QApplication(sys.argv)
    newTree = UndoableTree()
    newTree.show()
    sys.exit(app.exec_())    

if __name__ == "__main__":
    main()

Overall, this seems better than using clicked.