Should jj op restore be considered as part of the undo stack as well?

[Velociraptor115]

I've been looking at the implementations of the undo and redo stack, and it looks like it might be easy to support jj restore operations as part of the undo stack
Am I missing something intentional?

Currently, there's a difference between the following sequence of operations

Repo 1

#!/bin/sh
rm -rf repo1
mkdir repo1
pushd repo1

# Set up jj repo
# Also taking care to anonymize the repo
jj git init --no-colocate
jj config set --repo user.name "Test User"
jj config set --repo user.email "[email protected]"
jj abandon

jj commit -m "A"
jj commit -m "B"
jj commit -m "C"
jj commit -m "D"
jj op restore '@--'
jj commit -m "E"

jj undo
jj undo

Repo 2

#!/bin/sh
rm -rf repo2
mkdir repo2
pushd repo2

# Set up jj repo
# Also taking care to anonymize the repo
jj git init --no-colocate
jj config set --repo user.name "Test User"
jj config set --repo user.email "[email protected]"
jj abandon

jj commit -m "A"
jj commit -m "B"
jj commit -m "C"
jj commit -m "D"
jj undo
jj undo
jj commit -m "E"

jj undo
jj undo

The difference is jj op restore '@--' vs. jj undo (x2)

The outcome of both those operations are the same at that point
However, subsequent undo operations behave differently because jj op restore is not part of the undo / redo stack

The first puts you at D
The second puts you at A

[yuja]

It's intentional that we don't include explicit op restore/op revert operations in undo/redo stack. One can think that jj op restore OP && jj undo would revert the previous jj op restore OP operation.

cc @senekor

[Velociraptor115]

In that case, would it then make sense, from an user perspective to perform "n" undos in a single operation
For example, if a GUI/TUI shows the user the undo / redo stack, so that they can choose an operation to undo to

This is conceptually jj op restore, but the difference is that it would participate in the stack

Practically, it can be implemented by multiple invocations of jj undo or jj redo, but that allows possible interleaving operations by some other application
The alternative is to perform multiple "undo operations" within the same transaction

Or we can simply document that jj by itself will not concern itself with supporting this particular use case and it is up to the tools to implement it the best way possible

EDIT: My POV being that jj op restore can conceptually be treated as an "undo" operation, and the positive benefits would outweigh the behavioural change involved. I don't feel too strongly about it, so if it sounds like a bad idea, I'll just use one of the alternatives I mentioned above

[senekor]

Yeah, that was an intentional decision. I'm open to documenting this better.

My perspective when implementing the feature was that of trying to teach jj to people without any VCS experience. jj undo and jj redo currently behave very similar to equivalent features in typical GUI apps, so they are very easy to teach. Here's my tutorial chapter about them. It doesn't mention jj op commands at all.

The jj op commands work on a different level of abstraction. That's why the semantics shouldn't me mixed, in my opinion. I can think of some edge cases, where the semantics become confusing. Assume you run jj undo ; jj op restore @-. That op restore action should probably be considered a redo, not an undo. Also, operations can become "shadowed" in the sense that they're unreachable by running any number of jj undo and jj redo commands. But they can still be reached with jj op restore <precise-op-id>. That cannot be integrated with the undo-redo-stack logic.

So, while it's tempting to make them work together, since they are implemented very similarly, I think it's better to think of them as completely different commands.

[Velociraptor115]

Assume you run jj undo ; jj op restore @-. That op restore action should probably be considered a redo, not an undo. Also, operations can become "shadowed" in the sense that they're unreachable by running any number of jj undo and jj redo commands. But they can still be reached with jj op restore . That cannot be integrated with the undo-redo-stack logic.

Yeah, that makes sense