jamievicary / globular

Globular
37 stars 9 forks source link

Feature: allow undo without re-rendering #15

Open cdgls opened 8 years ago

cdgls commented 8 years ago

At the moment, clicking undo effectively reloads the entire project, in its previous state. For a large project this can take several minutes, and makes experimenting extremely slow and time-dangerous, as errors force an undo/reload. Is there a way to structure the undo operation so it doesn't re-render the entire project, but knows to only reload the working (ie open in the display space) cell in its previous state? (Maybe this requires separating the notion of "undo a modification to the working cell (ie in the display space)" from the notion of "undo a structural operation like the creation of a new cell or the deletion of a cell" (ie separate changes to the 'signature' from changes to the 'memory').)

jamievicary commented 8 years ago

The way that the 'undo' feature works is the following: every time you change something, the entire state of the project is backed up in the browser's history. So over time, a long chain of these backups is created, through which you can navigate.

Suppose by selecting an action, you change the state of the project from A to B. What we do at the moment is store the entire state A, allowing it to be retrieved if necessary. To make your vision a reality, we would instead need to encode information about how to modify B to create A.

So: I agree this would be very nice, and deserves to be done. But there's a lot of work needed to achieve it.

Also, once this is in place, for every new feature that is added, you would also need to add information on how to undo a step that makes use of it. So a certain overhead is added to the workflow for developing a new feature.

On Tue, Dec 15, 2015 at 7:52 PM, cdgls notifications@github.com wrote:

At the moment, clicking undo effectively reloads the entire project, in its previous state For a large project this can take several minutes, and makes experimenting extremely slow and time-dangerous, as errors force an undo/reload Is there a way to structure the undo operation so it doesn't re-render the entire project, but knows to only reload the working (ie open in the display space) cell in its previous state? (Maybe this requires separating the notion of "undo a modification to the working cell (ie in the display space)" from the notion of "undo a structural operation like the creation of a new cell or the deletion of a cell" (ie separate changes to the 'signature' from changes to the 'memory'))

— Reply to this email directly or view it on GitHub https://github.com/jamievicary/globular/issues/15.

jamievicary commented 8 years ago

A compromise, as you say, would be to only do this 'intelligent' undo operation for a certain subclass of operations (like rewriting or attaching to a diagram), and fall back to reloading an entire previous project state for other project changes.

On Tue, Dec 15, 2015 at 7:58 PM, Jamie Vicary jamievicary@gmail.com wrote:

The way that the 'undo' feature works is the following: every time you change something, the entire state of the project is backed up in the browser's history. So over time, a long chain of these backups is created, through which you can navigate.

Suppose by selecting an action, you change the state of the project from A to B. What we do at the moment is store the entire state A, allowing it to be retrieved if necessary. To make your vision a reality, we would instead need to encode information about how to modify B to create A.

So: I agree this would be very nice, and deserves to be done. But there's a lot of work needed to achieve it.

Also, once this is in place, for every new feature that is added, you would also need to add information on how to undo a step that makes use of it. So a certain overhead is added to the workflow for developing a new feature.

On Tue, Dec 15, 2015 at 7:52 PM, cdgls notifications@github.com wrote:

At the moment, clicking undo effectively reloads the entire project, in its previous state For a large project this can take several minutes, and makes experimenting extremely slow and time-dangerous, as errors force an undo/reload Is there a way to structure the undo operation so it doesn't re-render the entire project, but knows to only reload the working (ie open in the display space) cell in its previous state? (Maybe this requires separating the notion of "undo a modification to the working cell (ie in the display space)" from the notion of "undo a structural operation like the creation of a new cell or the deletion of a cell" (ie separate changes to the 'signature' from changes to the 'memory'))

— Reply to this email directly or view it on GitHub https://github.com/jamievicary/globular/issues/15.

cdgls commented 8 years ago

I guess I had hoped that the "state" of the project is quite neatly delineated into its "signature" (S) and its "active cell" (C) (the latter being what is in the main display space (or the stored source/target when building a new cell)) --- vaguely, the former being static, stored information and the latter being dynamic, in-memory information.

It seems like every operation either (modifies C) [type 1] or (modifies S and clears C) [type 2]. So clicking undo should [check if the last operation was type 1 or type 2; if type 2, reload the entire project in previous state; if type 1, reload only C in previous state and leave S unchanged].

I am of course speculating, but I wouldn't imagine this adds much development burden on new features (because any new feature that isn't of the existing type 1/2 would require huge structural changes under the hood that would dwarf this undo issue). Moreover, again speculatively, I'd actually think it encourages a valuable distinction around whether operations are type 1 or type 2.

cdgls commented 8 years ago

Yes, good point, the 'reload everything' can be a default in case an operation doesn't have an obvious 'miniundo' version.

akissinger commented 8 years ago

The way we handle this in quanto (and in fact a fairly standard way) is to maintain an "undo stack". Each item on the stack contains a little description of what happened (mainly for debugging or to show to the user) and a pair of "closures" (aka anonymous functions). One function, when called will perform the change forwards, whereas the other one will undo the change. For instance, deleting a node "a" is implemented something like this:

var n = getnode("a");
var item = {
  "desc": "Delete node",
  "redo": function() { delete("a"); },
  "undo": function() { var n1 = n; add("a", n1); } // storing 'n' in a local var may not be necessary, I don't know JS well enough to say for sure
}
item["redo"]();
undostack.push(item);
redostack.clear();

This can of course be wrapped in some helpful methods. For instance, the first thing you can implement is a "trivial" kind of undo item, which reverts a whole snapshot of the project just like you do now, then slowly replace these all with more fine-grained undo's.

The reason for storing both directions is it allows undo and redo. E.g. undo with:

item = undostack.pop();
item["undo"]();
redostack.push(item);

...and redo with:

item = redostack.pop();
item["redo"]();
undostack.push(item);
jamievicary commented 8 years ago

Yes, this is more or less what I was thinking. It's a neat idea to do it with callbacks, and to support 'redo' as well as 'undo'.

jamievicary commented 8 years ago

I just realized a potential problem with this. Suppose each time the state changes from S to S', we encode a minimal bit of information about how to modify S' to produce S. Then suppose multiple changes take place, so we go from S to S' to S''. Then suppose the user clicks and holds on their back button, and chooses the state S from 2 timesteps ago. The result will be nonsense, because the browser will receive information on how to turn S' into S, and will try to apply it to S''. I struggle to see how this can be overcome...