jj: `jj prev` moves back by two revisions if `@` has no children but is not empty

Description

Here’s an example:

jj log -T 'separate(" ",change_id.short(), description)'
@  umspkqwnpqvx add file2
◉  nprzlkrnkpuu add file1
◉  zzzzzzzzzzzz
jj prev
Working copy now at: otzuvlqz 21a57e68 (empty) (no description set)
Parent commit      : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 0 files, removed 2 files
jj log -T 'separate(" ",change_id.short(),description)'
@  otzuvlqzsrxt
│ ◉  umspkqwnpqvx add file2
│ ◉  nprzlkrnkpuu add file1
├─╯
◉  zzzzzzzzzzzz

Expected Behavior

I expect the state after jj prev to be this instead:

jj log -T 'separate(" ",change_id.short(), description)'
@  okxltnqrlpsr
│ ◉  umspkqwnpqvx add file2
├─╯
◉  nprzlkrnkpuu add file1
◉  zzzzzzzzzzzz

How to fix

I suppose it works this way because I do expect it to move by two if @ is empty:

jj log -T 'separate(" ",change_id.short(), description)'
@  umspkqwnpqvx add file2
◉  nprzlkrnkpuu add file1
◉  zzzzzzzzzzzz
jj new
jj log -T 'separate(" ",change_id.short(), description)'
@  uoqxurltlovk
◉  umspkqwnpqvx add file2
◉  nprzlkrnkpuu add file1
◉  zzzzzzzzzzzz
jj prev
Working copy now at: uvkovltz 265dc566 (empty) (no description set)
Parent commit      : nprzlkrn 6a17b2c5 add file1
Added 0 files, modified 0 files, removed 1 files
jj log -T 'separate(" ",change_id.short(), description)'
@  uvkovltzzxwm
│ ◉  umspkqwnpqvx add file2
├─╯
◉  nprzlkrnkpuu add file1
◉  zzzzzzzzzzzz

I think my expectation is that jj prev will move back by one if the @ revision will be abandoned once another revision is checked out (i.e. because it has no description changes). If the @ revision will not be abandoned, then jj prev should only move back by one.

About this issue

Original URL
State: closed
Created 3 months ago
Comments: 15 (1 by maintainers)

Commits related to this issue

Tweak the behavior of `jj prev` when `@` is a non-discardable tip Before this change, `jj prev` does this: ``` jj log -T 'separate(" ",change_id.short(), description)' @ umspkqwnpqvx add file2 ◉ np... — committed to martinvonz/jj by emesterhazy 3 months ago
Tweak the behavior of `jj prev` when `@` is a non-discardable tip Before this change, `jj prev` does this: ``` jj log -T 'separate(" ",change_id.short(), description)' @ umspkqwnpqvx add file2 ◉ np... — committed to martinvonz/jj by emesterhazy 3 months ago
Tweak the behavior of `jj prev` when `@` is a non-discardable tip Before this change, `jj prev` does this: ``` jj log -T 'separate(" ",change_id.short(), description)' @ umspkqwnpqvx add file2 ◉ np... — committed to martinvonz/jj by emesterhazy 3 months ago
Fix an issue where `jj prev` could not create a new revision on top of `@-` Before this change, `jj prev` does this: ``` jj log -T 'separate(" ",change_id.short(), description)' @ umspkqwnpqvx add f... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago
Improve the documentation of `jj prev` This will hopefully make it clear that `jj prev` does not move by [OFFSET] relative to `@`, which is a misconception that I had and I think others may also have... — committed to martinvonz/jj by emesterhazy 3 months ago

Most upvoted comments

I realized something important. Earlier you said:

It just does a jj new @- which is the root in your example.

This is wrong, isn’t it? It’s actually doing jj new @--. Why does it use @-- instead of @-? I suppose it’s because if @ is sitting on an empty commit then jj new @- is effectively a no-op. So like it or not, an empty @ commit is a special case that we need to handle for the command to be useful. The current implementation prioritizes the case where @ is empty at the expense of the case where @ is not. If it worked the other way around then someone would be complaining that jj prev doesn’t work when @ is empty.

Why do you not use jj new then

I mean, why does jj prev exist at all if you can do the same thing with jj new? Personally I find it kind of hard to type jj new @-. I think it has something to to with @ and - being far apart on the keyboard and requiring the shift key to be held. It’s certainly harder than typing jj prev, which I also have muscle memory for from mercurial.

This also starts to treat commits differently which is not a thing I want.

I really want to push back on the idea that this is a bad thing. I’m just another user like you said, but to me at least it really does seem like a completely empty @ commit created by jj new or jj commit is different than every other type of commit. If we didn’t treat it differently it would be an ergonomics problem since we’d all probably need to manually abandon lots of empty and unneeded commits created by moving the @ pointer around as we work on things. Completely empty commits are not the same as commits which contain information, and I don’t think treating the same way for the sake of consistency is a useful goal.

Edit:

The jj prev --help output even says that by default this moves backwards by one revision to the parent. That is not what happens in my example. In fact, unless you pass --edit, that’s never what happens if you don’t consider an empty @ commit to be a special case.

Usage: jj prev [OPTIONS] [AMOUNT]

Arguments:
  [AMOUNT]
          How many revisions to move backward. By default moves to the parent

          [default: 1]

emesterhazy on Apr 3, 2024

While a empty working copy is abandoned by other commands operating on the graph, I can’t see next and prev working as that…

jj prev already abandons the empty working copy. From another perspective you could say that the current behavior is inconsistent in that it doesn’t special case the empty working copy the way that many other actions do.

it’ll also break the current movement as we’d always need to adjust the working copy when moving backwards.

I don’t understand this point. What do you mean by “adjust the working copy”? I want jj prev to create a new commit on @- if @ isn’t an empty commit. jj prev --edit would move @ to @-. That seems fine to me? If you want to move to exactly @- then you can write that explicitly with jj new @- or jj edit @- instead of using prev.

This is purely a user perspective.

Can you expand on this? I usually view jj (the CLI program) through the lens of a human using the CLI. From that perspective I think this improves the usability of jj prev. Maybe you look at it through a different lens?

emesterhazy on Apr 2, 2024

I do not think this is a bug, it may be unexpected as the working copy is not treated as a “finished” commit but that is entirely accurate. It just does a jj new @- which is the root in your example.

PhilipMetzger on Apr 2, 2024