FR: jj undo ergonomics

[benbrittain]

Is your feature request related to a problem? Please describe.
The current behavior of the jj undo command does not seem to be what users intuitively expect. Currently if you run jj undo twice in a row it has the same behavior as jj op undo, it returns you to the same place you were when you started. undo + undo = no change.

Describe the solution you'd like
I'd like to preserve the ergonomics of the underlying jj op undo command (as suggested by @PhilipMetzger on Discord), but have the higher level jj undo command track if a user immediately undoes an undo and warn them.

Describe alternatives you've considered
Alternatively, the jj undo command could keep track of where you are in the undo log ctrl-Z style. That probably implies a redo command as well?

[martinvonz]

I'd like to preserve the ergonomics of the underlying jj op undo command

Why make them different? Oh, I suppose the top-level jj undo would lose its optional operation argument?

[benbrittain]

I'm actually starting to question if jj op undo is needed at all. I'd originally been modeling it as a lower level operation, but it's really just a special case of jj op restore, no? I can't imagine I'd ever use jj op undo if jj undo had ctrl-z semantics

[martinvonz]

it's really just a special case of jj op restore, no?

No, jj undo @- undoes the second-to-last operation while leaving the changes from the last operation. For example, if you abandoned some commit, and then did a bunch of unrelated changes, you can still recover that commit with jj undo <old operation>. It can be hard to reason about, however, because it also brings back ancestors of the old commit when it makes the old abandoned commit visible.

[martinvonz]

Think of it like jj backout but for operations (while jj op restore is jj restore for operations).

[PhilipMetzger]

To reword the FR a bit:

Improve the ergonomics of jj undo by making it a separate command, which can do undo tracking like editors. Currently running undo twice in a row, surprises a lot of users as it just rolls back the last operation (not surprising if you know that jj undo is an alias of jj op undo). This would then pave a way for a smart redo, which has the opposite behavior.

Describe alternatives you've considered
Alternatively, the jj undo command could keep track of where you are in the undo log ctrl-Z style. That probably implies a redo command as well?

I think that this alternative would be layering violation in UI terms, as imo jj op undo shouldn't be aware of such things anyway. That's where the suggestion came from in Discord anyway.

[joyously]

Would this have any impact on #3428 (op log waypoints)?

[PhilipMetzger]

Would this have any impact on #3428 (op log waypoints)?

The higher level jj undo could probably integrate with them, which then simplifies the UX again. op log Waypoints will just be another nice building block for it.

[fowles]

I think undo is the wrong idiom to think of these things with. The op log is a continuous stream of states that is append only. Attempts to imagine the current state as a pointer to somewhere in this history of operations is appealing, but in truth you are creating a new state at the head that happens to have identical content to an earlier state.

I would recommend removing undo entirely and instead building on restore as the fundamental unit. Give it good usability so you can say restore <timestamp> and then make it clear that what this command does it creates a new state now that has exactly the same contents as the system had at <timestamp>.

[martinvonz]

I would recommend removing undo entirely and instead building on restore as the fundamental unit.

Are you saying that jj op undo <not latest operation> is too hard to understand? I think that's fair. I practically never use it myself.

By the way, I find the suggestion of making jj undo behave differently from jj op undo confusing. That would be resolved by removing jj op undo. Another option is to rename jj op undo to jj op backout (matching jj backout just like jj op restore matches jj restore). Maybe we should start with that rename.

[PhilipMetzger]

The op log is a continuous stream of states that is append only. Attempts to imagine the current state as a pointer to somewhere in this history of operations is appealing, but in truth you are creating a new state at the head that happens to have identical content to an earlier state.

I agree with this, but still think that jj undo should be kept with the suggested semantics.

By the way, I find the suggestion of making jj undo behave differently from jj op undo confusing. That would be resolved by removing jj op undo. Another option is to rename jj op undo to jj op backout (matching jj backout just like jj op restore matches jj restore). Maybe we should start with that rename.

That sounds great and should make the implementation simpler as it can just shell out to op restore.

[ilyagr]

I have found jj op undo to be occasionally useful, but I like the idea of renaming it to jj op backout.

I'm not 100% sure what jj undo should do if we rename jj op undo to backout until we create a fancy new behavior. It would be a bit sad to get rid of it. I suppose it could be a version of jj op backout that only works on the last operation, and explains to the user that 1) repeating jj undo will undo the undo 2) try jj op log and jj op restore for anything fancy.

I am also not sure what a new and fancy jj undo would do, exactly. I wrote up a long explanation of how (I think) it worked in Emacs, but that made me realize that almost any text-editor-like undo behavior will run into the same problem: what if you do a bunch of undo-s, and then a non-user-initiated operation happens unexpectedly (like snapshotting from jj log running on a timer or because watchman notices something changed)?

---

Here's the write-up about the old Emacs behavior (or what I think it was)

I'd like to suggest the old Emacs behavior for consideration: if the op log is at

root -> A -> B -> C

the first jj undo undoes C, and the second jj undo undoes A. At that point, the log would be

root -> A -> B -> C -> undo C -> undo B

Now, if you did another undo, it would undo A. If you do any operation D other than undo (or redo), however, the op log becomes

root -> A -> B -> C -> undo C -> undo B -> D

Now, if you start undoing, you will first undo D, then you'll undo "undo B", then you'll undo "undo A", and so on.

The advantage of this is that it works well with an append-only op log, jj op log will tell you exactly what's going on.

There are some disadvantages, most notably: what do we do if "operation D" is an unexpected non-user-initiated operation, such as snapshotting?

[PhilipMetzger]

I am also not sure what a new and fancy jj undo would do, exactly. I wrote up a long explanation of how (I think) it worked in Emacs, but that made me realize that almost any text-editor-like undo behavior will run into the same problem: what if you do a bunch of undo-s, and then a non-user-initiated operation happens unexpectedly (like snapshotting from jj log running on a timer or because watchman notices something changed)?

I think we should be able to fix the Emacs use-case you mentioned with separately tagging the op log transactions to distinguish them from automation and human interaction, like #3428 wants. In my opinion we deserve an actual client/daemon for jj where direct forge integration and a VFS is built in and then having the option to tag where an op came from will be needed.

[emilazy]

Here’s a nice exposition of a completely linear undo feature that never loses history, even as you perform new changes on top of undos; all past states remain accessible at all times: Resolving the Great Undo-Redo Quandary. I don’t know if it would work for Jujutsu, but we should at least consider and internalize its lessons when thinking about this.

(It’s possible Emacs has behaviour similar to this; I’m not sure.)

[rdamazio]

Just spent a couple min trying to repeatedly jj undo to go back through the op log until I remembered that it was just going back and forth :) so +1 - at least in the simple case, repeated jj undo should find the first non-undo operation to undo.

[tim-janik]

Just spent a couple min trying to repeatedly jj undo to go back through the op log until I remembered that it was just going back and forth :) so +1 - at least in the simple case, repeated jj undo should find the first non-undo operation to undo.

jj-fzf has an implementation of jj undo along the jj op log: screencast
When you hit Alt+Z it will pick the first operation from the jj op log that is:
a) not an undo op itself,
b) has not been subject to a previous undo.
That way it effectively implements multi-step undo.
The op-log view (Ctrl+O) even shows the jj op log with undo/undone operations marked '-' instead of '○' so using that it is easy to see which operation will be the next subject to Alt+Z.

Since that makes it easy to test how far multi-step undo can be pushed, I tested it a couple of times and ran into cases that cannot be undone:

$ jj op log
[...]
○  0fe687841521 timj@fury 1 day ago, lasted 4 milliseconds
│  import git head
│  args: jj log -s -r ::@
○    efa2eba75c6a timj@fury 1 day ago, lasted 2 milliseconds
├─╮  reconcile divergent operations
│ │  args: jj --ignore-working-copy show --tool true '--color=always' -r @ -T '
│ │        "[@ " ++ concat(
│ │           separate(" ",
│ │             format_short_change_id_with_hidden_and_divergent_info(self),
│ │             format_short_commit_id(commit_id),
│ │             bookmarks,
│ │             if(conflict, label("conflict", "conflict")),
│ │           ),
│ │         ) ++ "]"'
○ │  bc183f57c1b7 timj@fury 1 day ago, lasted 16 milliseconds
│ │  new empty commit
│ │  args: jj new 77d57b3ff6c14698967f312be220c298f67f409f
○ │    e997ad6b277b timj@fury 1 day ago, lasted 7 milliseconds
├───╮  reconcile divergent operations
│ │ │  args: jj --ignore-working-copy show --tool true '--color=always' -r @ -T '
│ │ │        "...[@ ...]"'
○ │ │    037dc2bfb109 timj@fury 1 day ago, lasted 8 milliseconds
[...]
$ jj op undo efa2eba75c6a69a0
Error: Cannot undo a merge operation

So unless there was a way to reliably linearize the op log, endless undo (redo) doesn't really work in practice.
Or could the logic I mentioned above be extended to ignore additional steps and continue undo beyond this point?

[jennings]

I would recommend removing undo entirely and instead building on restore as the fundamental unit.

I think it would be a mistake to remove undo. It is so common for newcomers to express their delight when they learn they can jj undo anything.

jj undo is easy to understand with nearly no training (besides that it only works once). jj op restore requires one to first understand what the op log means.

Let people get excited by jj undo, then I think they'll learn jj op restore when they hit its limitations. Better ergonomics like jj op restore --to "monday" would still be great additions.

The op log is a continuous stream of states that is append only. Attempts to imagine the current state as a pointer to somewhere in this history of operations is appealing, but in truth you are creating a new state at the head that happens to have identical content to an earlier state.

Is this a fundamental requirement of the op log? I don't know the data structures so I don't know if it's actually feasible, but if the "current operation" could be a pointer to an old operation, instead of always being the head, then I think undo/redo would be way easier to handle:

• jj undo would move the op@ pointer back by one operation (with some assistance if there are two parents).
• jj redo would move the op@ pointer forward by one operation (with some assistance if there are two children).

This would create the semantics I think people expect:

• jj undo always goes "back in time"
• jj redo works as long as the last change was a jj undo
• If you commit a new operation after a few undos, jj redo stops working (since you're now at a head) and jj undo would now undo that operation, then continue backing up from there.
  • If you did this by mistake, you could still recover with jj op restore

If it's not feasible, then I think we'd essentially need to store the undo tree alongside the op log to achieve the following:

• jj undo "skips over" undo operations. If you have operations A->B->C->@ and @ is an undo of C, then jj undo actually restores to A.
• jj redo only works if @ is an undo and has similar skipping behavior: A -> B -> C -> @ where @ is an undo of C (and therefore equivalent to B), then jj redo restores to A.

[rdamazio]

And btw, just emitting a warning when you jj undo something that was itself an undo operation would probably go a long way to prevent confusion, even if we don't change the ergonomics :)

[martinvonz]

Is this a fundamental requirement of the op log?

I would say that it is. @tim-janik just reminded me of this message from me: #3455 (comment). I think that provides some useful context.

[ilyagr]

I'm wondering if a variation of @jennings 's idea might be viable:

Let's start with the oplog A->B->C. (So, @=C initially)

If the current operation is not undo/redo, jj undo works as it now does. Perhaps we would have it print something like:

$ jj undo
Undid operation C: frobnicate the repo
Do `jj undo --again` to undo operation B: prepare for frobnication

Now, our oplog consists of 4 operations: A->B->C->"undo C", and the current operation is @="undo C".

When the current operation is an undo, I'd forbid jj undo entirely (this is somewhat tangential to the rest of the plan) and require the user to do jj undo --again (better names?) to continue the sequence of undoes. jj undo --again would read the description of the current operation, "undo C", and undo the parent of C, B in this case. As normal (and as we do now), this will be a new operation on top of the operation log. So, then the current operation would be "undo B", and another jj undo --again would undo "A".

jj undo --again would fail if the current operation is not undo/redo.

We could also have jj redo that is only allowed if the current operation is an undo or a redo. It would see that the current operation undoes A and redo A (this is in fact equivalent to what jj undo does now if the top operation is an undo, but I find this simpler to think about if we record this operation as "redo A" in the oplog instead of "undo undo"). After such a redo, jj undo --again¹ would undo the operation A again, describing the operation as "undo A" (a new operation on top of the op log, like any other operation).

The motivation for requiring --again if the top operation is actually mostly to disallow jj undo --again if the top operation is not an undo, but some concurrent or unexpected operation that breaks a chain of undoes. I don't want the user to unexpectedly undo such an operation; instead they should be informed of the problem. If the unexpected operation doesn't interfered with what they were undoing, they can look up the last successful jj undo, and then do jj undo <the-parent-of-last-undone-operation>, which could be told to them by an error message (by scanning the oplog for the last undo operation).

Update: To relate this back to Stephen's idea, conceptually this stores a pointer to another oplog operation inside the oplog record of the undo operation. We could keep the oplog the same as it is now, and parse the undo pointer from the description of the oplog operation, or we could add a new field to the oplog proto that would only be populated for undo/redo operations and point to the operation being undone/redone.

Footnotes

1. After jj redo, "--again" is not a great name, but I don't have a better idea; "--chain" seems worse.

   Instead of --again, another option would be to require jj undo --head or jj undo <rev> to start a sequence of undoes, and have a plain jj undo do what I describe as jj undo --again. This would be a more breaking change, but otherwise might be better? ↩

[joyously]

I sort of like Ilya's plan, but not this part

When the current operation is an undo, I'd forbid jj undo entirely (this is somewhat tangential to the rest of the plan)

because the default would be @ and that should make sense, and novices are the ones most likely to do undo twice so it should have an intuitive result, and sometimes you just want to undo the undo.
If the undone operation is a rebase or split or whatever, does undoing it propagate the changes (as I expect it would)? If so, the old commits still exist? (I don't really understand when garbage collection is done.) I'm wondering if the redo uses the old commit or makes new ones.

[jennings]

@ilyagr's solution seems reasonable to me.

The motivation for requiring --again if the top operation is actually mostly to disallow jj undo --again if the top operation is not an undo, but some concurrent or unexpected operation that breaks a chain of undoes. I don't want the user to unexpectedly undo such an operation; instead they should be informed of the problem.

I see this concern, but I don't think the user will appreciate the difference. If I understand correctly, only one of jj undo and jj undo --again can ever succeed, so it feels a bit like, "Nuh-uh, you didn't say the magic word."

• If the user accidentally creates a new operation while making an undo chain, then they're probably fine undoing that, because by running jj undo again we know their intention is to keep going back in time.
• If the user purposefully creates a new operation, then it's reasonable to expect them to know that jj undo will now undo that change.

jj undo could instead look at the head op and automatically do the "again" logic if it was an undo operation.

jj undo --again would read the description of the current operation, "undo C", and undo the parent of C, B in this case.

The initial version could parse the operation description, but we might eventually want this to be some kind of structured metadata, either part of the operation itself or linked to it (like git notes).

[tim-janik]

In the meantime, I have updated the jj-fzf undo implementation following @jennings suggestion to use a marker instead of the (slightly involved) procedure I described earlier:

• Each time Alt+Z is pressed in the jj-fzf op-log, jj undo is executed for the operation that is the parent of the last undo operation executed. Then this op is saved via jj config set --repo jj-fzf.last-undo $OP_ID.
• There is a check in place, so that the undo operation to be carried out is only taken to be the parent of the jj-fzf.last-undo marker, if the marker matches the topmost (last) operation being undone. Otherwise jj undo works as usual and the topmost op will be undone.

That alone works well enough for reliable multi-step undo, as long as the operation history is linear. If the operation history fans out (due to concurrent repository changes), there is no reliable way to reconstruct older repository states, AFAICS. And jj undo simply refuses to operate at that point.

Below is a screenshot of the op log in jj-fzf after 8 consecutive undo operations (all marked ⋯ together with their respective undo operations). We have been having that in place for a couple weeks and the experiences so far are:
a) While it might be nice to have multi-step undo, in the vast majority of cases a single step is sufficient. Except for the cases where a script action needs to be undone - since scripts (like jj-fzf) sometimes do jj new -B && jj squash or similar multi-step operations.
b) To confidently engage in multi-step undo, you really want to have a live look at the op log, just to figure out at what point your repository is at in the history of modifications. If you think you need 4, 5 or more undo steps, you probably are better of using restore instead.
c) In the very rare case where you want to redo a previous undo step, it feels odd to require some intermediate modifying operation (jj new or describe) just to "reset" the undo marker. That's why I added Alt+K to reset the marker in the op log, so the next Alt+Z is effectively a redo of the last undo.

All in all, I'm quite happy how undo/restore work in the jj-fzf op log view now. As for the jj undo CLI ergonomics, what I would suggest is:

• Move the above described marker logic into the jj CLI and move jj-fzf.last-undo into jj core. This could look like adding jj undo --parent-of-last-undo which uses and updates the marker.
• After a while you might want to make that the default jj undo behavior.
• Add something along the lines of jj undo --reset-last-undo-marker.

And then, leave it at that. As stated above, more than a handful of successive undo ops are probably best left to UI tools with a live view, or are an indication that you probably want to use jj restore / jj op restore.
Don't bother with attempts to flatten the operation history, at least not for jj undo when you generally only need a handful of consecutive steps anyway. And, as Martin keeps repeating, that is not what it was designed for anyway:
The operation log allows to detect and merge divergent operations

[robey]

I like the article posted in this comment above: #3700 (comment) -- I think the current operation log in jj is pretty close to what it's describing.

The one major change to jujutsu would just be what the other comments have been saying. To summarize:

• jj undo should move the undo pointer backward through the op log.
• jj redo should move it forward.
• The end state of running jj undo 3 times should be viewing the state of the repo as it was 3 operations before starting to undo.

I softly disagree with the idea that it's okay for the command-line to be hard to understand, because GUI tools will "pick up the slack". This is one of the many problems that makes git irritating to both newbies and power users. The CLI tools can be "basic" but they should be simple, understandable, and complete. If they're hard to understand or use, people may bounce before getting around to trying GUI tools.

[martinvonz]

I like the article posted in this comment above: #3700 (comment) -- I think the current operation log in jj is pretty close to what it's describing.

I didn't understand the proposal in that article. In particular, why does the "type one character; undo" sequence double the undo stack?

• jj undo should move the undo pointer backward through the op log.

I thought the article suggested recording the undo as its own operation instead of moving the pointer back (like we already do). Also see my comment above.

I softly disagree with the idea that it's okay for the command-line to be hard to understand, because GUI tools will "pick up the slack".

I think we all agree.

[robey]

I didn't understand the proposal in that article. In particular, why does the "type one character; undo" sequence double the undo stack?

Admittedly I didn't try to understand the specific encodings & optimizations they were using for their text editor. I read it as an enhancement of "canonical undo/redo" which allows undo-of-an-undo.

This may be the wrong place for a lengthy description, but I was talking to a UX friend about it this morning, and it seems like word processors perform undo/redo like this:

insert "the" --> insert "sleeping" --> insert "cat"
                                       ^
                                       current document
                                       "the sleeping cat"

After 2 undo (command-Z):

insert "the" --> insert "sleeping" --> insert "cat"
^
current document
"the"

Redo (shift-command-Z) twice will still work to bring back "the sleeping cat", or if you start typing "purring" here, the redo stack is lost forever, and you have:

insert "the" --> insert "purring"
                 ^
                 current document
                 "the purring"

I think the GURQ proposal is to behave the same way except to track the undo operations at the end (as jujutsu already does).

commit A --> commit B --> squash
                          ^
                          current tree

After two "undo" commands, the tree is back at the first commit:

commit A --> commit B --> squash --> undo squash --> undo commit B
^
current tree

Doing a new operation bumps the undo pointer to the end of the operation log again so that future undo/redo will treat the previous undo's (undone's?) as operations themselves, which can be navigated through.

commit A --> commit B --> squash --> undo squash --> undo commit B --> commit C
                                                                       ^
                                                                       current tree

• jj undo should move the undo pointer backward through the op log.

I thought the article suggested recording the undo as its own operation instead of moving the pointer back (like we already do). Also see my comment above.

Yes, I think it does both:

• undo/redo navigates forward/backward through the history ("canonical undo/redo")
• if new work happens after an undo, the undo is recorded in the history, and the new work recorded after that (as jj currently does)

[martinvonz]

Thanks for explaining. I still don't understand the exponential stuff, but maybe we don't need to understand it.

One idea is to make jj undo record in the operation that it represents an undo. Then the next jj undo will search the operation log backwards and count how many undos it finds before the first non-undo. It will then restore 2*n+1 operations back.

Perhaps a more reliable way is to record which operation was undone by which other operation. Then we would instead scan backwards until we find an operation that's not an undo and is not itself undone by a later operation.