Undo/redo - HUGE problem (possibly fixed, but leaving issue open)

[jimfinnis]

It seems that Undo/Redo is pretty much not working at all. Perhaps it's a regression - it must be, or I would never have said undo was basically done.

Currently, when recreating the nodes of the previous state, we have to delete the previous graph. This causes all tabs to close. But all sorts of weird stuff happens too - the nodes still seem to think they have a tab open? Crashes ensue, or problems such as being unable to reopen a node's tab.

Repro:

• open a new doc
• create an expr node
• open a tab on it
• change output type to ROI
• click undo. You now have two expr nodes occupying the same place, one with the previous state (might not be obvious, try moving them).
• undo again. Tab closes, you still have that extra node.

[jimfinnis]

Temporarily disabled UNDO.

[jimfinnis]

OK, here's what was happening. Firstly, the code which deleted the existing nodes:

for n in self.nodes:
    n.remove()

had problems, in that the remove() will remove from self.nodes, causing modify-during-traverse problems. Fixed with a copy. Frankly I'm embarrassed. I think this must be a regression, because that's daft code. This would cause rogue copies of nodes to hang around. I think I must have become too used to sane languages which would throw an exception if you did that.

Secondly (and less importantly) the remove() would also close any open tabs for that node - we don't want that to happen. Instead, when we undo we want to deserialise new nodes and remove old nodes WITHOUT closing their tabs, so that MainUI.replaceDocument() can repatch the tabs to point to the new set of nodes (and vice versa). Thus a flag called "closetabs" which defaults to true has been added to methods down that path.

Things seem better. I'll leave the issue open, though.