how do I record changes to undo/redo?

[wmayer]

This should fix the issue: 7c708eeca3

[uwestoehr]

This should fix the issue: 7c708eeca3

Many thanks! It works nicely.

So from my point of view the only missing bit is that the property “AutoTransactionView” should be there and true by default so that new FC users (fresh installation) get the same Undo behavior, no matter if they change a feature property in the Data or the View tab.

[Kunda1]

So from my point of view the only missing bit is that the property “AutoTransactionView” should be there and true by default so that new FC users (fresh installation) get the same Undo behavior, no matter if they change a feature property in the Data or the View tab.

great question, where would we add this in the wiki for newb devs ? So they utilize when they’re first studying and writing code for FC?

[openBrain]

great question, where would we add this in the wiki for newb devs ? So they utilize when they’re first studying and writing code for FC?

Not sure devs shall be addressed. Because they have to code the transactions themselves in any case.
For users, Fine-tuning and eventually Std_Undo are IMO the good pages.

[wmayer]

Besides this, I am still not understanding why bug 4552 occurs. This one is definitely my mistake but I cannot find a way to fix it.

The different behaviour is caused by the logic of the constructors of TaskDimension and TaskBalloon.

In TaskDimension it first sets the values to the widgets and then connects their signals to a slot – which is good doing it this way.
In TaskBalloon for one widget it first connects the signal to a slot and then changes the value. This triggers the slot and changes the feature and the automatic transaction system spots this and creates a new undo transaction – which is bad behaviour.

Although the behaviour of the balloon dialog made the undo working for you it’s not the right way to do and it has been properly fixed with bb0d75b6c and 3288c02eba

[uwestoehr]

Although the behaviour of the balloon dialog made the undo working for you it’s not the right way to do and it has been properly fixed with bb0d75b6c and 3288c02eba

Thanks!.
But I am still stuck because now all GUI properties changed by the dimension dialog are recorded for undo, but not the APP properties. So now you can e.g. change the fontsize in the Dimension dialog and undo that change. But changing a tolerance setting in the dialog cannot be undone.

[wmayer]

But I am still stuck because now all GUI properties changed by the dimension dialog are recorded for undo, but not the APP properties. So now you can e.g. change the fontsize in the Dimension dialog and undo that change. But changing a tolerance setting in the dialog cannot be undone.

Works fine for me.

1. Create a std, box
2. Create a techdraw page and add the box
3. Select an edge and set a length dimension. It may display the value 9.12
4. Double click the dimension and set Over tolerance to 0.1. The item may show 9.12±0.1
5. Confirm with OK
6. Click on undo => The dimension will be 9.12 again

[uwestoehr]

Works fine for me.

Not for me. I recompiled now everything but the problem persists. Take this example file:
Würfel von Reinhard.FCStd (75.5 KB)
It does not work:

[uwestoehr]

[*]Create a std, box…

Also with your recipe it doesn’t work. Maybe the issue is only triggered by certain features I use. Here is the file I created using your recipe:
TestForWerner.FCStd (24 KB)

[uwestoehr]

Also for the Balloon dialog Undo is not working as it should. I took the box example from my previous post and just added a balloon:

So I first created the balloon, then I changed the fontsize in a second action. But one press on Undo undoes both actions at once so i end up without any balloon.

[wmayer]

Not for me. I recompiled now everything but the problem persists. Take this example file: Würfel von Reinhard.FCStd

After closing the dimension dialog what does the undo list show as top item when you click on the little black arrow? In my case it shows “Dimension bearbeiten”.

[wmayer]

Also with your recipe it doesn’t work. Maybe the issue is only triggered by certain features I use. Here is the file I created using your recipe:
TestForWerner.FCStd

OK, I just realized that you double-click the item in the TechDraw view while I double-clicked the tree item.

[wmayer]

Also with your recipe it doesn’t work. Maybe the issue is only triggered by certain features I use. Here is the file I created using your recipe:
TestForWerner.FCStd

OK, I just realized that you double-click the item in the TechDraw view while I double-clicked the tree item.

The main problem why it doesn’t work as expected is due to issues of the class QGIDatumLabel. It overrides the methods mousePressEvent() and mouseReleaseEvent() to emit the signal dragFinished(). This signal is connected to datumLabelDragFinished() that opens and commits a transaction and violates with the implementation of mouseDoubleClickEvent(). The result is that when the dimension dialog appears the active transaction has been closed and that’s why it fails to store the made changes.

To me it looks like mousePressEvent() and mouseReleaseEvent() are the wrong event handlers to emit the signal because even if you only select an item two transactions appear in the undo list.

Another side-effect is that it touches the document while when you select via the tree view it doesn’t touch the document and no transaction is created either.

The correct event handler actually should be dragLeaveEvent().

[wmayer]

This fixes the undo/redo of the dimension: a82a103cb3

[wmayer]

This fixes the undo/redo of the balloon: 008e97b929

One thing for balloon should actually be changed: when you move a balloon item it generates hundreds of undo actions which then quickly exceeds the limit. It makes more sense to only create an undo action after releasing the mouse. Therefore this code block should be moved to balloonLabelDragFinished().

[ceremcem]

This question reminds me of @ceremcem’s post Making projects resistant to breaking changes https://forum.freecadweb.org/viewtopic.php?f=8&t=54298

ping

Yes, directly related. If we had an option to record every step as a macro, there were absolutely zero issues about any number of undo/redo operations.

[uwestoehr]

This fixes the undo/redo of the balloon: 008e97b929

many thanks!

One thing for balloon should actually be changed: when you move a balloon item it generates hundreds of undo actions which then quickly exceeds the limit.

Thanks. Did not yet notice this. This bug destroys the undo capability, so dragging a balloon you cannot undo other thing → nasty bug whose fix should go in for 0.19.

Here is the PR that fixes this for me: https://github.com/FreeCAD/FreeCAD/pull/4456
I purposely did there not also temporarily redraw the OriginX/Y since while dragging you cannot change X/Y and also the OriginX/X. However, I did not authored the balloon feature. Maybe there is a case that I just don’t know in which the origin could change during a label drag, I kept its recomputation for balloonLabelDragFinished() as safe guard.