Warn/require confirmation on bookmarking empty changes?

[drieber]

Discussed in #5363

^{Originally posted by sourcefrog January 14, 2025}
In getting to know jj, when I am working on a feature branch that I want to push to github, I've fairly often run a flow like

jj commit -m Whatever
jj bookmark move featurebranch

The trap of course is that the bookmark is set to the new empty revision following the thing that I committed. I suppose pushing the empty rev to a feature branch on github is not too harmful but it does seem a bit messy.

The right answer I guess is that I should either

1. Move the bookmark before committing, which seems a bit weird coming from other VCS
2. bookmark move -r@-
3. Just jj desc then bookmark it but then don't forget to jj new

I see there is a good bit of existing discussion about various types of auto-moving bookmarks, which would perhaps be a more fundamental answer for this kind of workflow:

• Feedback for the experimental `advance-branches` feature #3549
• Working branches and the JJ "way" #2425
• How bookmarks and Git ought to interact #5333
• FR: Topics (alternative to branches) #3402
• FR: --advance-branches flag for jj commit #2338

As a more concrete and tactical (perhaps excessively tactical) fix I wonder if the UI should by default prevent moving a bookmark to an empty change, the same way it by default prevents you pushing new bookmarks and moving bookmarks backwards.

That is, jj new; jj bookmark set bbb would fail unless you said --allow-empty.

Alternatively, rather than failing it could say

warning: bookmark bbb is set to an empty revision
hint: to set it to the parent revision use `jj bookmark move bbb -rrstuvwxyz --allow-backwards`

[drieber]

I am working on fixing this issue.

[martinvonz]

I agree with @yuja's suggestion here, i.e. that we should just make jj bookmark create/set/move require a revision argument instead of defaulting to @.

[drieber]

I agree with @yuja's suggestion here, i.e. that we should just make jj bookmark create/set/move require a revision argument instead of defaulting to @.

That sounds good to me. There are two practical things to consider:

• Many tests do jj bookmark create (and probably also set/move) without specifying a revision. Those tests need to change.
• Some users may actually prefer the current behavior (an implicit -r@).

Do we want to introduce a new jj config knob(s), say:

• bookmark-create.target_revision_defaults_to_working_copy = false
• bookmark-set.target_revision_defaults_to_working_copy = false
• bookmark-move.target_revision_defaults_to_working_copy = false

That way users (or tests) can set those knows to true to continue working as they do today. I don't have a strong preference, and don't mind fixing every test if necessary.

[martinvonz]

Good question. We can ask on Discord how people feel about it. I basically never create bookmarks (outside of tests and demos).

[arxanas]

Regardless of what we do with the default, I'm against adding configuration for this. It seems like too much UI complexity for minimal gain.

Also regardless of the default, shouldn't we just warn whenever you try to set a bookmark to an empty, single-parent revision?

• It's not clear to me if the proposal is to warn only for the default case, or in all cases.
• Even if done explicitly, it seems likely to be a mistake (one of those things you have to adjust to when coming from Git).

Philosophically, I think we should remove the default, as it seems to derive from the working-copy-oriented branching style, in order to keep commits reachable/visible. If users prefer to bookmark @ by default can't they add an alias for it, anyways?

[martinvonz]

shouldn't we just warn whenever you try to set a bookmark to an empty, single-parent revision?

I think it's fine to set a bookmark to such commits, kind of like how it's okay to set a description before you write code. By making the revision argument required, we're just making the user think about the target, without saying that it's a bad thing to target an empty commit. But that's just my opinion. Note that I barely use bookmarks myself.

The discussion on Discord seemed a bit inconclusive. It seemed like some users like the current default. Some users would prefer if it was @-. It seemed like no one felt strongly either way. So I'm not sure where that leaves us. Maybe we just leave it as is for now?

[sourcefrog]

I think it's a bit of a trap for new users onboarding to the jj model, especially because putting the bookmark back on @- takes a somewhat long command. Requiring an explicit destination seems coherent with jj requiring explicit instructions for some other cases. But that's just my impression as a new user.

[drieber]

I agree that making target revision explicit is a good change. Specifically, jj bookmark create will return an error if target revision is not explicitly given on the command-line (I will later deal with bookmark set and bookmark move).

I agree with martinvonz that we should allow setting a boomark on an empty commit, but we can revisit that decision. I view that as separate from this open issue.

[kastl-ars]

I think it's a bit of a trap for new users onboarding to the jj model, especially because putting the bookmark back on @- takes a somewhat long command. Requiring an explicit destination seems coherent with jj requiring explicit instructions for some other cases. But that's just my impression as a new user.

+1 for being consistent, i.e. having the commands require explicitly specifying a revision.

As for new users, the warning about moving bookmarks backwards does not make the experience any better, so avoiding that seems like a good idea.

And I think users that are accustomed to the current behaviour are able to work around that with e.g. an alias as recommended above.

[martinvonz]

As I said above, the conclusion from Discord was unclear. But the recent messages here all seem to be for making the revision argument required :) @jennings, I think you were one of the users on Discord who said you kind of like the current behavior. I know you said you don't feel very strongly at all, so I'm not calling you here to defend your position (but feel free to) :) I Just wanted to make sure it won't be a surprise, given the Discord discussion, if we decide to make the revision argument required.

I'm fine with making the revision argument required.

+1 for being consistent, i.e. having the commands require explicitly specifying a revision.

I'm for being consistent, but I'm not convinced that requiring an explicit revision argument is consistent. Lots of commands currently default to @ (abandon, absorb, describe, diff, diffedit, new, ...).

As for new users, the warning about moving bookmarks backwards does not make the experience any better, so avoiding that seems like a good idea.

I think that's a separate issue. I would like to hear from more users before we make a decision about that. Feel free to open a separate issue for removing that flag (I think you're talking about --allow-backwards).

[kastl-ars]

As for new users, the warning about moving bookmarks backwards does not make the experience any better, so avoiding that seems like a good idea.

I think that's a separate issue. I would like to hear from more users before we make a decision about that. Feel free to open a separate issue for removing that flag (I think you're talking about --allow-backwards).

You misunderstood me, sorry. I was merely adding to @sourcefrog 's point that the command to move a bookmark backwards is long. Most newcomers will try to do that and then get that "scary" warning about having to use the --allow-backwards option.

As for the discussion on whether or not bookmark commands would need an explicit revision or not:

Maybe the better way out for now is to address the topic of setting bookmarks to empty changes as described in the OP. Maybe introducing a configuration option to have jj allow or not allow setting a bookmark to an empty change (or have jj warn or not warn) would be an easy way out?

[ilyagr]

the command to move a bookmark backwards is long

In case people don't know, --allow-backwards has a short alias, -B. Currently, the only way to find out about that is to run jj bookmark move --help. Perhaps we should include the short version in the message as well.

The reason it's not there is that it's a bit difficult to insert both options without making the message more confusing, but hopefully we'll think of something. (I think we have to include the long version since it explains what the option does)

Perhaps "Use --allow-backwards (alias -B) to allow it."

[joyously]

I don't think there should be a limit on which revision can be bookmarked.
The problem seems to be that bookmarks are not branches, yet that's what they are used for, so perhaps bridging the gap in the jj git push command is the answer.
Would it make sense to have an option on push to do what a Git user expects (move the bookmark and push what changed)?
Or maybe a revset for changes in the range of bookmark to tip (probably already doable, but I don't know and novices don't know)?

I think you can't get rid of the Git expectations since there is a Git interface, but there should be less friction when using it.

[jennings]

I usually edit commits directly instead of new+squash, and I am usually editing the last commit in a stack anyway when I'm ready to push. So, that's why I use jj b set with no -r flag.

Even though it's ever so slightly inconvenient for my use, it sounds like requiring -r is the right call because:

• It's a possible foot-gun for newcomers.
• Someone using the squash workflow would never want the default, and it seems like we encourage the squash workflow by default.
• Adding -r@ is not a huge burden.
• Making it optional again in the future is a non-breaking change.

If users prefer to bookmark @ by default can't they add an alias for it, anyways?

Yes, but I think aliases for sub-commands are a little inconvenient because I have to backtrack my thoughts. This is my thought process step-by-step:

# Time to set a bookmark!

$ jj _

# Bookmark commands are nested under the "bookmark" namespace
$ jj bookmark _

# Oh crap, I have an alias for this
^H^H^H^H^H^H^H^H

$ jj bset my-branch <ENTER>

If I could create an alias nested under the "bookmark" command, then I'd have no issue with it :)