cli: add `--untrack` option to `bookmark forget` #5546

scott2000 · 2025-02-01T00:22:58Z

Fixes #5524. I went with --untrack for the naming because it seemed easier to explain in the help text than --local.

Checklist

If applicable:

I have updated CHANGELOG.md
I have updated the documentation (README.md, docs/, demos/)
I have updated the config schema (cli/src/config-schema.json)
I have added tests to cover my changes

When untracking a remote bookmark, I usually also want to forget the local bookmark associated with it, and there is no way to do this in a single command currently. Instead, it's necessary to either untrack the remote bookmark first and then delete the local bookmark, or forget the bookmark first and then run `jj git fetch` to correct the remote bookmark which is now missing. Instead of adding `--untrack` to `jj bookmark forget`, we could add a `--delete-local` option to `jj bookmark untrack`. This might be more unintuitive in cases where there are multiple tracked remote bookmarks though, since a user might forget to stop tracking one remote bookmark, and then the deleted local bookmark might accidentally be pushed to that remote. Therefore, it seems safer to add the flag here, since it's clear that it untracks all of the remote bookmarks.

yuja · 2025-02-01T01:24:00Z

iirc, @ilyagr had some idea about jj branch forget options.

My feeling is that jj branch forget --untrack sounds like an option to do more things than the default, but actually it is an option to restrict the target scope, which is surprising.

ilyagr · 2025-02-03T05:25:32Z

Thank you for working on this!

I have thought of this operation as jj forget --local, since you forget only the local bookmark. We once discussed this on Discord and Yuya preferred untrack --all-remotes back then. I think I still prefer forget --local, but either that or untrack --all-remotes seem better than forget --untrack.

(BTW, I'm aware that #5524 exists, but I haven't read it carefully at this point, sorry if I'm repeating things already discussed there)

The reason I prefer forget --local is that I also want jj forget --remote branch@remote that would forget a remote-tracking branch without forgetting the local branch (jj forget --remote 'glob:branch@*' would also be allowed). One use-case for this is when I fetched more branches than I wanted from upstream and want to undo this mistake.

We don't have to implement it in this PR, but this UI with --local and --remote makes sense to me . This results in the trichotomy of three modes for jj forget:

Forget the local branch and all remote-tracking branches (the current behavior and the default)
Forget some or all of the remote-tracking branches, but keep the local one.
Forget only the local branch without marking the remote-tracking branches for deletion (this PR)

The confusion is that "forget the local branch and untrack all the remote-tracking branches" doesn't fit into this pattern quite as well, but the fact that it's equivalent to 3. as I described above is good enough for me.

ilyagr · 2025-02-03T05:42:22Z

cli/src/commands/bookmark/forget.rs

@@ -33,6 +33,10 @@ use crate::ui::Ui;
 /// recreated on future pulls if it still exists in the remote.
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkForgetArgs {
+    /// Untrack remote bookmarks instead of forgetting them


Whether this is called "--untrack" or "--local", here's an idea for the help text:

/// Forget chosen local bookmarks. Untrack (but do not forget) the corresponding remote bookmarks.

ilyagr · 2025-02-03T05:43:21Z

cli/src/commands/bookmark/untrack.rs

@@ -26,6 +26,9 @@ use crate::ui::Ui;
 ///
 /// A non-tracking remote bookmark is just a pointer to the last-fetched remote
 /// bookmark. It won't be imported as a local bookmark on future pulls.
+///
+/// If you want to forget a local bookmark while also untracking the
+/// corresponding remote bookmarks, use `jj bookmark forget --untrack` instead.


Nice idea to mention it here! (No action needed, beyond updating the name if we change it)

yuja · 2025-02-03T07:50:37Z

I have thought of this operation as jj forget --local, since you forget only the local bookmark. We once discussed this on Discord and Yuya preferred untrack --all-remotes back then. I think I still prefer forget --local, but either that or untrack --all-remotes seem better than forget --untrack.

(Perhaps, untrack --all-remotes wouldn't delete local bookmark, but I don't remember why forget --local sounded weird.)

If I redesign forget from scratch, I would probably make:

forget NAME deletes matching local bookmarks and disassociates them from tracking remotes (i.e. forget --local/--untrack)
forget NAME@REMOTE deletes matching remote bookmarks
forget --all[-remotes] NAME deletes matching local and remote bookmarks (the current behavior)

I think the tricky part of forget --local is that it actually updates per-remote state.

scott2000 · 2025-02-03T13:38:14Z

The reason I prefer forget --local is that I also want jj forget --remote branch@remote that would forget a remote-tracking branch without forgetting the local branch (jj forget --remote 'glob:branch@*' would also be allowed). One use-case for this is when I fetched more branches than I wanted from upstream and want to undo this mistake.

We don't have to implement it in this PR, but this UI with --local and --remote makes sense to me.

Yeah --local does make a lot more sense if there's also going to be a --remote option as well.

If I redesign forget from scratch, I would probably make:

forget NAME deletes matching local bookmarks and disassociates them from tracking remotes (i.e. forget --local/--untrack)

forget NAME@REMOTE deletes matching remote bookmarks

forget --all[-remotes] NAME deletes matching local and remote bookmarks (the current behavior)

I like this a lot better than the current behavior. It's always felt strange to me that if I pass the name of a local bookmark, it forgets the remote bookmarks too. I would be in favor of changing it to this or something like this.

I think the tricky part of forget --local is that it actually updates per-remote state.

Yeah, I think that's what made me want to go with --untrack instead of --local, since it seems weird to me that --local would change the behavior of the command for remote bookmarks without changing the behavior for local bookmarks.

martinvonz · 2025-02-03T21:22:16Z

Yuya's proposal makes sense to me too. I'm not at all attached to how it currently behaves.

ilyagr · 2025-02-03T22:06:26Z

I also like Yuya's proposal. I think the state the repo is in after jj forget --local is usually more desireable than the state after the current jj forget, since it is not messed up by a subsequent fetch. So, it makes sense to make that the default.

My only nit (that I'm sure we can figure out) is that I don't like the --all-remotes name for the option to represent the current behavior, since I'd expect that to only forget the remote-tracking branches. The best name I came up with so far is --with-all-remotes (the short can still be --all) or --also-forget-remotes, or maybe --forget-remotes¹. Another option that came to mind is --everywhere, but that's slightly optimistic since it doesn't forget the branch on actual remote servers.

I'm conflicted about this one. In theory, it shares the same problem as --all-remotes, but it's somehow easier for me to imagine --forget-remotes meaning "do the usual thing and also forget remotes". ↩

ilyagr · 2025-02-03T22:10:35Z

There's also the question of how to make this transition. Do you think it's OK to make this breaking change directly?

At some point, I had the plan to require at least one of --local and --remote for jj forget in a transitional period. Then, I thought we'd make --local the default.

At that point, I was thinking of the syntax as being jj forget br --remote someremote instead of jj forget br@someremote or jj forget --remote br@someremote.

I think we could still have an intermediate period where we'd require an argument when specifying the local branch, either jj forget --local br or jj forget --with-all-remotes br. Instead of --remote, we can simply allow jj forget br@someremote.

Or we can just make the breaking change directly, I'm not sure how confusing this would be. I imagine it'd have the most impact on GUIs, since I'm guessing not many people use jj forget in scripts. But that guess could be wrong.

scott2000 · 2025-02-03T22:17:27Z

@ilyagr

Maybe another option would be to just say that if someone wants to forget the remote bookmark, they have to do so explicitly. Like jj bookmark forget bookmark bookmark@remote to forget both at the same time instead of jj bookmark forget --with-all-remotes bookmark. I'm really struggling to think of any use case where it's necessary to forget both local and remote bookmark at the same time, and also untracking isn't sufficient. But if we want to avoid making a breaking change, it does seem like it would be useful to support it still.

joyously · 2025-02-03T22:22:57Z

I'm really struggling to think of any use case where it's necessary to forget both local and remote bookmark at the same time, and also untracking isn't sufficient.

I was just going to ask for all the scenarios this is trying to cover, so I could get a better feel for the option that would make sense.
I think there are different cases when you have only one remote versus multiple remotes. Also a factor is your workflow (stacking changes on main, or pushing single revisions to your fork). And if you fetch other authors' work, you want to forget more bookmarks.

yuja · 2025-02-04T00:53:11Z

I don't like the --all-remotes name for the option to represent the current behavior, since I'd expect that to only forget the remote-tracking branches.

Agreed. It could also mean forget --all-remotes (without name patterns).

How about --include-remotes? I don't think this option is useful, but something like this would be needed for smoother migration. I think breaking change is acceptable. bookmark forget wouldn't be widely used.

scott2000 · 2025-02-04T03:02:46Z

How about --include-remotes? I don't think this option is useful, but something like this would be needed for smoother migration. I think breaking change is acceptable. bookmark forget wouldn't be widely used.

I like it! I can take a shot at implementing it sometime this week, but if someone else would like to do it instead that's fine with me too.

ilyagr reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add `--untrack` option to `bookmark forget` #5546

cli: add `--untrack` option to `bookmark forget` #5546

scott2000 commented Feb 1, 2025

yuja commented Feb 1, 2025

ilyagr commented Feb 3, 2025 •

edited

Loading

ilyagr Feb 3, 2025 •

edited

Loading

ilyagr Feb 3, 2025

yuja commented Feb 3, 2025

scott2000 commented Feb 3, 2025

martinvonz commented Feb 3, 2025

ilyagr commented Feb 3, 2025

ilyagr commented Feb 3, 2025 •

edited

Loading

scott2000 commented Feb 3, 2025

joyously commented Feb 3, 2025

yuja commented Feb 4, 2025

scott2000 commented Feb 4, 2025

cli: add --untrack option to bookmark forget #5546

Are you sure you want to change the base?

cli: add --untrack option to bookmark forget #5546

Conversation

scott2000 commented Feb 1, 2025

Checklist

yuja commented Feb 1, 2025

ilyagr commented Feb 3, 2025 • edited Loading

ilyagr Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

ilyagr Feb 3, 2025

Choose a reason for hiding this comment

yuja commented Feb 3, 2025

scott2000 commented Feb 3, 2025

martinvonz commented Feb 3, 2025

ilyagr commented Feb 3, 2025

Footnotes

ilyagr commented Feb 3, 2025 • edited Loading

scott2000 commented Feb 3, 2025

joyously commented Feb 3, 2025

yuja commented Feb 4, 2025

scott2000 commented Feb 4, 2025

cli: add `--untrack` option to `bookmark forget` #5546

cli: add `--untrack` option to `bookmark forget` #5546

ilyagr commented Feb 3, 2025 •

edited

Loading

ilyagr Feb 3, 2025 •

edited

Loading

ilyagr commented Feb 3, 2025 •

edited

Loading