Undo/Redo creation/deletion of plots breaks when code in the editor is changed

[bjkomer]

If you create a component (value plot, slider, etc) and then modify the model in the code editor (i.e. remove a connection or ensemble, doesn't even have to be directly related to where the component came from), and then undo and redo the component creation, things get messed up. The component will get created again, but with a uid something like <Ensemble (unlabeled) at 0x7f7235a98e50> and throw a key error.

[s72sue]

I am unable to repro this one. Which browser are you using?

[bjkomer]

This was in Firefox, but I just tried it in Chrome and it is happening there too.

Here's what I did

• open net.py
• add a value graph for the ens ensemble
• comment out the connection from d to a on line 26
• press undo, and then press redo

It looks like part of my comment got cut off (apparently < and > mess things up), the uid I'm seeing is something like: Ensemble (unlabeled) at 0x7f1ba7097e50

[tcstewar]

Ah... that uid is what happens when it can't find an item. So the undo system might be able to detect that this has happened....

[bjkomer]

There would have to be a lot of detecting going on. We also run into problems if someone moves an ensemble, then removes that ensemble from the code editor, and then tries to undo it. Is there some sort of resetting going on that changes uids when code in the editor is changed? I'm just not sure how it is not finding the item, it even draws the graph back in the correct location just before throwing the error. I need to look into it a bit more.

What should the correct approach be to undoing an action that cannot be performed? One option is to detect it when it comes up, and just skip over it (I think this should be straight forward in every case except for auto-layout). Another could be to figure out what changed in the code and strip out all items in the stack with matching uids (this sounds messy and prone to bugs). A third option would be to just delete the undo stack when you change the code (simplest thing to do, but not a very good option).

[tcstewar]

Is there some sort of resetting going on that changes uids when code in the editor is changed? I'm just not sure how it is not finding the item, it even draws the graph back in the correct location just before throwing the error.

The reload system in components/netgraph.py tries its hardest to figure out what the matching uids are when things change, but it's a tricky problem.

What should the correct approach be to undoing an action that cannot be performed? One option is to detect it when it comes up, and just skip over it (I think this should be straight forward in every case except for auto-layout). Another could be to figure out what changed in the code and strip out all items in the stack with matching uids (this sounds messy and prone to bugs). A third option would be to just delete the undo stack when you change the code (simplest thing to do, but not a very good option).

I'm curious what option 1 would be like -- that might feel kinda okay. Option 3 is also at least clear and straight-forward, so we may have to fall back on that.

The coolest option (4) would be to integrate with ace's undoManager. That might actually be doable... http://ace.c9.io/api/undomanager.html

[tcstewar]

Better docs for that, including a discussion of inserting your own actions into the undo stack:

http://cloud9-sdk.readme.io/v0.1/docs/the-undomanager

[tcstewar]

It's definitely a big task to merge the two undo stacks, but I think it'd be worthwhile to at least fix this so that it doesn't break, and do the merging in a separate PR. Even something as simple as clearing the netgraph's undo stack whenever a change happens in the code editor would be a good temporary measure.