I haven't found the cause yet but in this solution if we complete a relationship and then move one of the handles and press any key we get an : "assert not self._current_transaction" when doing tx_data.commit()

Thanks for testing 🙏 . I'll try to reproduce it.

To continue on this issue I discovered that the problem is when doing tool.reset() to a drag. If we don't call the on_drag_end defined in gaphas in itemtool.py then we don't get the error.

To continue on this issue I discovered that the problem is when doing tool.reset() to a drag. If we don't call the on_drag_end defined in gaphas in itemtool.py then we don't get the error.

That's odd. It doesn't look like on_drag_end is doing anything out of the ordinary.

Something else occurred to me. If you change from normal item tool to a placement tool, all controllers on the view widget are removed and new ones are added. This could (partially) explain why the drag controller was missing end signals. It may be that we need to prevent controllers from being swapped out when in a transaction.

It can also be that replacing controllers is a bad idea altogether. Instead we should add a limited set of controllers only once and change tool behavior from within those controllers.

I created a new diagram with 2 classes, connected by an association.
Now move one end of the association, and hit the right mouse button while dragging.
Gaphor will not react to click events anymore.

@0lione I cannot reproduce the issue you describe with tool.reset().

I added an extra check so tools (event controllers) are not swapped while a transaction is active. Now right-click while dragging does no longer make gaphor unresponsive to mouse events.

I noticed, though, that If I cancel a grag action, an entry is added to the undo stack. This should not happen.

I added more updates, mostly on undo management:

• Consolidate undo/redo stack handling. Basically 4 things can happen:
  1. A transaction has to be added to the undo stack (clear redo stack).
  2. A transaction is undone: remove from undo stack and add to redo stack.
  3. A transaction is redone: remove from redo stack and add to undo stack.
  4. A transaction is rolled back: do not change the undo and redo stack.
• Add a second phase for transaction rollback events, similar to commit events
• Remove the workaround to allow for certain events to happen outside a transaction. Since diagrams are now completely updated in a transaction, there's no need for this.
• Documentation updates

@0lione @danyeaw Still I ended up having transactions for cancelled operation on the undo stack. This is because events are queued if they are fired from within an event handler. This is done to preserve the order of events.

I changed tactics a bit and now set the transaction context (or intent). This allows me to differentiate between normal, undo, redo, and rollback transactions.

I think the cancel/undo behavior is pretty stable now. More stable than it has ever been. It needs a round of thorough testing, though.

@0lione Thanks again for helping out. You're changes helped uncover some deeper issues in the undo functionality.

@all-contributors Add @0lione for code.

@amolenaar

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

I've put up a pull request to add @0lione! 🎉

@0lione Thanks again for helping out. You're changes helped uncover some deeper issues in the undo functionality.

@all-contributors Add @0lione for code.

Thanks it was a pleasure to contribute in this amazing project.

@0lione

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

@0lione already contributed before to code