Undo/Redo implementation, can't connect to a button, it works by itself!

valeSimu

I post the question also in stackoverflow, "here":http://stackoverflow.com/questions/28195457/qt-undo-redo-implementation-can-t-connect-to-a-button

I am trying to implement an undo and redo commands in my application. I have a QTreeWidget and I'd like to let the user undo and redo an action (ex. change a value in a QTreeWidgetItem columns in the QTreeWidget and undo/redo it).

Here part my code:

class A.h

@class A : public QWidget
{
Q_OBJECT
public:
explicit A(...);
~A();

ChangeValueTreeCommand      *commands;
QUndoStack                  *undoStack;
QPushButton                 *undoBtn;
QPushButton                 *redoBtn;
QString                      newValue;

void changeItem(QTreeWidgetItem* treeWidgetItemChanged, int col);

};
@

class A.cpp

@
A::A(...){
undoStack = new QUndoStack(this);
}

void A::changeItem(QTreeWidgetItem* treeWidgetItemChanged, int col){

 ....
commands = new ChangeValueTreeCommand(treeWidgetItemChanged, col, newValue);
connect(undoBtn, SIGNAL(clicked()), commands, SLOT(undo()));
undoStack->push(commands);

}
@

class Commands.h

@

#ifndef COMMANDS_H
#define COMMANDS_H

#include <QApplication>

#include <QUndoCommand>
#include <QTreeWidgetItem>

class ChangeValueTreeCommand : public QUndoCommand
{
public:
explicit ChangeValueTreeCommand(QTreeWidgetItem* treeWI = NULL, int c = 0, const QString changedV = "");
~ChangeValueTreeCommand();

QTreeWidgetItem* treeWItem;
const QString    changedValue;
int col;

public slots:

void undo();
void redo();

};

#endif // COMMANDS_H
@

class Commands.cpp

@

#include "Commands.h"

ChangeValueTreeCommand::ChangeValueTreeCommand(QTreeWidgetItem* treeWI, int c, const QString changedV)
: treeWItem(treeWI), col(c), changedValue(changedV)

{}

ChangeValueTreeCommand::~ChangeValueTreeCommand(){}

void ChangeValueTreeCommand::undo()
{
const QString oldValue = treeWItem->text(col);
treeWItem->setText(col, oldValue);
}

void ChangeValueTreeCommand::redo()
{
treeWItem->setText(col, changedValue);
}
@

The problem is that when the user changes the value in the QTreeWidgetItem, it automatically appears the previous value. Moreover, I would like to connect the undo and redo functions to two buttons, but the compiler says that

bq. 1543: error: C2664: 'QMetaObject::Connection QObject::connect(const QObject *,const char *,const QObject *,const char *,Qt::ConnectionType)'ÿ: cannot convert parameter 3 from 'ChangeValueTreeCommand *' to 'const QObject *'
Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

Can someone help me? Thx

raven-worx

In your undo() you get the current text of the item and re-set it. What do you actually expect? :)
@
void ChangeValueTreeCommand::undo()
{
const QString oldValue = treeWItem->text(col);
treeWItem->setText(col, oldValue);
}
@
You need to store the current text when you create the command. And use this for restoring.

Regarding the buttons: QUndoStack also provides ready-to-use QActions you can e.g. place in a QMenu or QToolBar.

QUndoStack ::createRedoAction()
QUndoStack ::createUndoAction()

If thats not an option for you then please show us the connect statement which gives you the error.

euchkatzl

The value changes because when you push a undo command to your undo Stack it's redo function is immediately called (look at the documentation for QUndoStack (http://doc.qt.io/qt-5/qundostack.html#push) ).

I don't know where you get your newValue of your ChangeValueTreeCommand. So that could be the reason. Try to debug it. I think you will find that mistake.

For a proper implementation you have to do a little undo Stack locking
@void A::changeItem(QTreeWidgetItem* treeWidgetItemChanged, int col){

 if(undoStackIsLocked() /* You have to implement that function on your own*/ ){
        // set here value of QTreeWidgetItem
 }else{
     commands = new ChangeValueTreeCommand(treeWidgetItemChanged,col, newValue);

     undoStack->push(commands);
 }

}@

The Problem of your connection is that you have to connect slots from your undo Stack !! So in you constructor do the following :
@A::A(...){
undoStack = new QUndoStack(this);
connect(undoBtn, SIGNAL(clicked()), undoStack, SLOT(undo()));
}@

I haven't tested that code. Hope it helps anyway.

valeSimu

thank you all, now my undo() works, and also the connection between the button and the undo() slot!

For the first problem I did like this (thanks raven-worx)

treeWItem->setText(col, oldValue);

and the second problem I did how euchkatzl said.

Now my problem is the redo(), apparently it is called when I push ChangeValueTreeCommand in the undoStack. Apparently I have to lock it, do you have any idea on how to do it? thank you!

raven-worx

[quote author="valeSimu" date="1422539525"]
Now my problem is the redo(), apparently it is called when I push ChangeValueTreeCommand in the undoStack. Apparently I have to lock it, do you have any idea on how to do it? thank you![/quote]
Thats default behavior. And there is no flag to prevent this. You need to redesign your implementation that actually the push of the command to the QUndoRedo stack is the action.

Otherwise there are only rather dirty hacks like setting a bool variable to false initially and set it to true in redo() and only execute the code in redo() when the variable is true:

@
void redo()
{
if( doRedo == false )
{
doRedo = true;
return;
}
//redo code here
}
@