Switch undo storage from a tree to a plain list #4777

occivink · 2022-11-12T15:32:15Z

See commit message for details.
I've not tested too extensively, but I figured I would put a PR out already so that others can test if they wish. From my brief experimentation it seems like good change.

I was wondering if it might not be interesting to optimize the 'reverse' node. Instead of computing ^C at the moment that we commit a new group, we could have nodes be some kind of union between a full node with the full content, or a 'reference' node with just an node index + a boolean flag (since a reference might be a ^C, or a ^^C). The inverse would be computed on demand, which I think would save some memory.
edit: after reading a bit about the storage of the undo modification, StringDataPtr looks to be reference counted and so there should be no memory duplication in case significant buffer modifications (for example pasting large text).
There might still be some benefit in using the approach described above, but not as large I thought.

~~If this is accepted, I will adapt the documentation later.~~

TeddyDD · 2022-11-30T00:26:57Z

I've been running this branch locally for two weeks. The linear history is great. Less mental overhead when undoing.

arrufat · 2022-11-30T12:27:47Z

If this gets merged, maybe we can pick up the newly freed <a-u> and <a-U> for undoing and redoing selection changes (#4725). What do you think @krobelus?

mawww · 2022-12-01T23:44:13Z

I'll run with this merged for a while to get a feel for it.

krobelus · 2022-12-08T12:09:46Z

If this gets merged, maybe we can pick up the newly freed and for undoing and redoing selection

I used to think so but I realized that selection undo is much closer to the jump list than to content undo. With #4807 <c-o> and selection undo might be used together, so maybe ctrl is better. Will try both.

I don't really use the inactive branches of undo history so I don't have an opinion on this one.
On a related note, maybe it's worth allowing the jump list (and selection history) to go back to any previous state, just like with content undo. Probably hard to come up with an intuitive UI. Today when I want to preserve a jump list, I open a new client which works alright.

Today, selection undo and jump list commands only travel the active branch of the selection history. Let's make it possible to visit branches that were undone and abandoned. I'm not sure how the user interface should look like. For now, let's make the existing shortcuts traverse the history tree in-order. This is equivalent to the undo behavior proposed in mawww#4777 THe upside of this UI is that it's dead simple to use because it needs no additional keys. The potentially annoying thing is that it requires to press <c-i>/<c-o>/<c-h>/<c-k> more often in some scenarios.

krobelus · 2022-12-08T19:31:29Z

I've implemented the equivalent behavior for selection undo and the jump list.
If anyone wants to try, it's in 03bfbe0 where I've also included this PR.

EDIT: I'll push newer versions to branch nonlinear-selection-undo.

Today, selection undo and jump list commands only travel the active branch of the selection history. Let's make it possible to visit branches that were undone and abandoned. I'm not sure how the user interface should look like. For now, let's make the existing shortcuts traverse the history tree in-order. This is equivalent to the undo behavior proposed in mawww#4777 THe upside of this UI is that it's dead simple to use because it needs no additional keys. The potentially annoying thing is that it requires to press <c-i>/<c-o>/<c-h>/<c-k> more often in some scenarios.

Today, selection undo and jump list commands only travel the latest branch of the selection history. If I undo a selection change and then make another change, the undone change cannot be redone - it lives on an old branch of the selection history. Let's make it possible to revisit such selections. There are a couple of ways to implement this. This version allows to toggle between redo branches whenever there is more than one. Hopefully this is not too hard to use. A previous approach implemented a behavior equivalent to mawww#4777, but that turns out to be clunky for selection undo because when I use <c-o>, I typically only want to travel the active branch and not revisit any abandoned branches. Another approach would be to mimic <a-u>. This week, I finally realized how that command works: it simply decrements the index of the current history item. Since the history tree is stored in a vector, this allows to visit any state. However this also means that it includes weird jumps between branches in the history. I somehow thought that it only jumps between the leaf nodes.. Anyway, I find this a bit unintuitive.

krobelus · 2022-12-26T06:17:57Z

Ok for selection undo this behavior is probably not right because I often want to do

jump to A
jump to B
jump to C
use <c-o> to jump back to B
jump to D
use <c-o> to jump back to B
use <c-o> to jump back to A

with the list behavior, step 7 jumps to D first; so it would required two extra <c-o>.
For selection history it's important to make excursions cheap. My new approach to navigate the undo tree is in #4816 (still experimental).

Not sure if this applies to content undo. I find that tree navigation more intuitive than current <c-u>.. will see which one is easy to use in practice

Today, selection undo and jump list commands only travel the latest branch of the selection history. If I undo a selection change and then make another change, the undone change cannot be redone - it lives on an old branch of the selection history. Let's make it possible to revisit such selections. There are a couple of ways to implement this. This version allows to toggle between redo branches whenever there is more than one. Hopefully this is not too hard to use. A previous approach implemented a behavior equivalent to mawww#4777, but that turns out to be clunky for selection undo because when I use <c-o>, I typically only want to travel the active branch and not revisit any abandoned branches. Another approach would be to mimic <a-u>. This week, I finally realized how that command works: it simply decrements the index of the current history item. Since the history tree is stored in a vector, this allows to visit any state. However this also means that it includes weird jumps between branches in the history. I somehow thought that it only jumps between the leaf nodes.. Anyway, I find this a bit unintuitive.

Whenever a new history node is committed after some undo steps, instead of creating a new branch in the undo graph, we first append the inverse modifications starting from the end of the undo list up to the current position before adding the new node. For example let's assume that the undo history is A-B-C, that a single undo has been done (bringing us to state B) and that a new change D is committed. Instead of creating a new branch starting at B, we add the inverse of C (noted ^C) at the end, and D afterwards. This results in the undo history A-B-C-^C-D. Since C-^C collapses to a null change, this is equivalent to A-B-D but without having lost the C branch of the history. If a new change is committed while no undo has been done, the new history node is simply appended to the list, as was the case previously. This results in a simplification of the user interaction, as two bindings are now sufficient to walk the entire undo history, as opposed to needing extra bindings to switch branches whenever they occur. The <a-u> and <a-U> bindings are now free. It also simplifies the implementation, as the graph traversal and branching code are not needed anymore. The parent and child of a node are now respectively the previous and the next elements in the list, so there is no need to store their ID as part of the node. Only the committing of an undo group is slightly more complex, as inverse history nodes need to be added depending on the current position in the undo list. The following article was the initial motivation for this change: https://github.com/zaboople/klonk/blob/master/TheGURQ.md

mawww · 2023-04-18T00:37:59Z

Heads up: I've been running with this for a while, and I think I'll merge it soon.

occivink force-pushed the undo-history-as-plain-list branch 2 times, most recently from 0ab5dc4 to 486616a Compare November 13, 2022 09:28

occivink force-pushed the undo-history-as-plain-list branch from 486616a to e0d33f5 Compare April 17, 2023 08:36

mawww merged commit a4918f9 into mawww:master Apr 24, 2023

Screwtapello mentioned this pull request Apr 24, 2023

Persistent Undo #2021

Open

sidkshatriya mentioned this pull request May 3, 2023

%val{history} does not seem 1-based #4885

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch undo storage from a tree to a plain list #4777

Switch undo storage from a tree to a plain list #4777

occivink commented Nov 12, 2022 •

edited

Loading

TeddyDD commented Nov 30, 2022

arrufat commented Nov 30, 2022 •

edited

Loading

mawww commented Dec 1, 2022

krobelus commented Dec 8, 2022

krobelus commented Dec 8, 2022 •

edited

Loading

krobelus commented Dec 26, 2022

mawww commented Apr 18, 2023

Switch undo storage from a tree to a plain list #4777

Switch undo storage from a tree to a plain list #4777

Conversation

occivink commented Nov 12, 2022 • edited Loading

TeddyDD commented Nov 30, 2022

arrufat commented Nov 30, 2022 • edited Loading

mawww commented Dec 1, 2022

krobelus commented Dec 8, 2022

krobelus commented Dec 8, 2022 • edited Loading

krobelus commented Dec 26, 2022

mawww commented Apr 18, 2023

occivink commented Nov 12, 2022 •

edited

Loading

arrufat commented Nov 30, 2022 •

edited

Loading

krobelus commented Dec 8, 2022 •

edited

Loading