cli: untrack remote bookmarks on bookmark forget

Forgetting remote bookmarks can lead to surprising behavior since it
causes the repo state to become out-of-sync with the remote until the
next `jj git fetch`. Untracking the bookmarks should be a simpler and
more intuitive default behavior. The old behavior is still available
with the `--include-remotes` flag.

I also changed the displayed number of forgotten branches. Previously
when forgetting "bookmark", "bookmark@remote", and "bookmark@git" it
would display `Forgot 1 bookmarks`, but I think this would be confusing
with the new flag since the user might think that `--include-remotes`
didn't work. Now it shows separate `Forgot N local bookmarks` and
`Forgot M remote bookmarks` messages when applicable.

Author: scott2000

CHANGELOG.md

@@ -25,6 +25,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   `Commit`. All methods on `Commit` can be accessed with `commit.method()`, or
   `self.commit().method()`.
 
+* `jj bookmark forget` now untracks any corresponding remote bookmarks instead
+  of forgetting them, since forgetting a remote bookmark can be unintuitive.
+  The old behavior is still available with the new `--include-remotes` flag.
+
 ### Deprecations
 
 * This release takes the first steps to make target revision required in

cli/src/commands/bookmark/delete.rs

@@ -30,6 +30,9 @@ use crate::ui::Ui;
 /// revisions as well as bookmarks, use `jj abandon`. For example, `jj abandon
 /// main..<bookmark>` will abandon revisions belonging to the `<bookmark>`
 /// branch (relative to the `main` branch.)
+///
+/// If you don't want the deletion of the local bookmark to propagate to any
+/// tracked remote bookmarks, use `jj bookmark forget` instead.
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkDeleteArgs {
     /// The bookmarks to delete

cli/src/commands/bookmark/forget.rs

@@ -26,13 +26,22 @@ use crate::command_error::CommandError;
 use crate::complete;
 use crate::ui::Ui;
 
-/// Forget everything about a bookmark, including its local and remote
-/// targets
+/// Forget a bookmark without marking it as a deletion to be pushed
 ///
-/// A forgotten bookmark will not impact remotes on future pushes. It will be
-/// recreated on future pulls if it still exists in the remote.
+/// If a local bookmark is forgotten, any corresponding remote bookmarks will
+/// become untracked to ensure that the forgotten bookmark will not impact
+/// remotes on future pushes.
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkForgetArgs {
+    /// When forgetting a local bookmark, also forget any corresponding remote
+    /// bookmarks
+    ///
+    /// A forgotten remote bookmark will not impact remotes on future pushes. It
+    /// will be recreated on future fetches if it still exists on the remote. If
+    /// there is a corresponding Git-tracking remote bookmark, it will also be
+    /// forgotten.
+    #[arg(long)]
+    include_remotes: bool,
     /// The bookmarks to forget
     ///
     /// By default, the specified name matches exactly. Use `glob:` prefix to
@@ -57,22 +66,36 @@ pub fn cmd_bookmark_forget(
     let repo = workspace_command.repo().clone();
     let matched_bookmarks = find_forgettable_bookmarks(repo.view(), &args.names)?;
     let mut tx = workspace_command.start_transaction();
+    let mut forgotten_remote: usize = 0;
     for (name, bookmark_target) in &matched_bookmarks {
         tx.repo_mut()
             .set_local_bookmark_target(name, RefTarget::absent());
         for (remote_name, _) in &bookmark_target.remote_refs {
-            tx.repo_mut()
-                .set_remote_bookmark(name, remote_name, RemoteRef::absent());
+            // If `--include-remotes` is specified, we forget the corresponding remote
+            // bookmarks instead of untracking them
+            if args.include_remotes {
+                tx.repo_mut()
+                    .set_remote_bookmark(name, remote_name, RemoteRef::absent());
+                forgotten_remote += 1;
+                continue;
+            }
+            // Git-tracking remote bookmarks cannot be untracked currently, so skip them
+            if jj_lib::git::is_special_git_remote(remote_name) {
+                continue;
+            }
+            tx.repo_mut().untrack_remote_bookmark(name, remote_name);
         }
     }
-    writeln!(ui.status(), "Forgot {} bookmarks.", matched_bookmarks.len())?;
-    tx.finish(
-        ui,
-        format!(
-            "forget bookmark {}",
-            matched_bookmarks.iter().map(|(name, _)| name).join(", ")
-        ),
+    writeln!(
+        ui.status(),
+        "Forgot {} local bookmarks.",
+        matched_bookmarks.len()
     )?;
+    if forgotten_remote != 0 {
+        writeln!(ui.status(), "Forgot {forgotten_remote} remote bookmarks.")?;
+    }
+    let forgotten_bookmarks = matched_bookmarks.iter().map(|(name, _)| name).join(", ");
+    tx.finish(ui, format!("forget bookmark {forgotten_bookmarks}"))?;
     Ok(())
 }
 

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` instead.
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkUntrackArgs {
     /// Remote bookmarks to untrack

cli/tests/[email protected]

@@ -290,7 +290,7 @@ See the [bookmark documentation] for more information.
 
 * `create` — Create a new bookmark
 * `delete` — Delete an existing bookmark and propagate the deletion to remotes on the next push
-* `forget` — Forget everything about a bookmark, including its local and remote targets
+* `forget` — Forget a bookmark without marking it as a deletion to be pushed
 * `list` — List bookmarks and their targets
 * `move` — Move existing bookmarks to target revision
 * `rename` — Rename `old` bookmark name to `new` bookmark name
@@ -322,6 +322,8 @@ Delete an existing bookmark and propagate the deletion to remotes on the next pu
 
 Revisions referred to by the deleted bookmarks are not abandoned. To delete revisions as well as bookmarks, use `jj abandon`. For example, `jj abandon main..<bookmark>` will abandon revisions belonging to the `<bookmark>` branch (relative to the `main` branch.)
 
+If you don't want the deletion of the local bookmark to propagate to any tracked remote bookmarks, use `jj bookmark forget` instead.
+
 **Usage:** `jj bookmark delete <NAMES>...`
 
 ###### **Arguments:**
@@ -336,11 +338,11 @@ Revisions referred to by the deleted bookmarks are not abandoned. To delete revi
 
 ## `jj bookmark forget`
 
-Forget everything about a bookmark, including its local and remote targets
+Forget a bookmark without marking it as a deletion to be pushed
 
-A forgotten bookmark will not impact remotes on future pushes. It will be recreated on future pulls if it still exists in the remote.
+If a local bookmark is forgotten, any corresponding remote bookmarks will become untracked to ensure that the forgotten bookmark will not impact remotes on future pushes.
 
-**Usage:** `jj bookmark forget <NAMES>...`
+**Usage:** `jj bookmark forget [OPTIONS] <NAMES>...`
 
 ###### **Arguments:**
 
@@ -350,6 +352,12 @@ A forgotten bookmark will not impact remotes on future pushes. It will be recrea
 
    [wildcard pattern]: https://jj-vcs.github.io/jj/latest/revsets/#string-patterns
 
+###### **Options:**
+
+* `--include-remotes` — When forgetting a local bookmark, also forget any corresponding remote bookmarks
+
+   A forgotten remote bookmark will not impact remotes on future pushes. It will be recreated on future fetches if it still exists on the remote. If there is a corresponding Git-tracking remote bookmark, it will also be forgotten.
+
 
 
 ## `jj bookmark list`
@@ -485,6 +493,8 @@ Stop tracking given remote bookmarks
 
 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` instead.
+
 **Usage:** `jj bookmark untrack <BOOKMARK@REMOTE>...`
 
 ###### **Arguments:**

cli/tests/test_bookmark_command.rs

@@ -512,16 +512,12 @@ fn test_bookmark_forget_glob() {
     let (stdout, stderr) =
         test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "glob:foo-[1-3]"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 2 bookmarks.
-    "###);
+    insta::assert_snapshot!(stderr, @"Forgot 2 local bookmarks.");
     test_env.jj_cmd_ok(&repo_path, &["undo"]);
     let (stdout, stderr) =
         test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "glob:foo-[1-3]"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 2 bookmarks.
-    "###);
+    insta::assert_snapshot!(stderr, @"Forgot 2 local bookmarks.");
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  bar-2 foo-4 230dd059e1b0
     ◆   000000000000
@@ -534,9 +530,7 @@ fn test_bookmark_forget_glob() {
         &["bookmark", "forget", "foo-4", "glob:foo-*", "glob:foo-*"],
     );
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 1 bookmarks.
-    "###);
+    insta::assert_snapshot!(stderr, @"Forgot 1 local bookmarks.");
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  bar-2 230dd059e1b0
     ◆   000000000000
@@ -705,13 +699,17 @@ fn test_bookmark_forget_export() {
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "export"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @"");
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "foo"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "forget", "--include-remotes", "foo"],
+    );
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 1 bookmarks.
-    "###);
-    // Forgetting a bookmark deletes local and remote-tracking bookmarks including
-    // the corresponding git-tracking bookmark.
+    insta::assert_snapshot!(stderr, @r#"
+    Forgot 1 local bookmarks.
+    Forgot 1 remote bookmarks.
+    "#);
+    // Forgetting a bookmark with --include-remotes deletes local and
+    // remote-tracking bookmarks including the corresponding git-tracking bookmark.
     insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
     let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r=foo", "--no-graph"]);
     insta::assert_snapshot!(stderr, @"Error: Revision `foo` doesn't exist");
@@ -763,8 +761,11 @@ fn test_bookmark_forget_fetched_bookmark() {
     ");
 
     // TEST 1: with export-import
-    // Forget the bookmark
-    test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "feature1"]);
+    // Forget the bookmark with --include-remotes
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "forget", "--include-remotes", "feature1"],
+    );
     insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
 
     // At this point `jj git export && jj git import` does *not* recreate the
@@ -797,7 +798,10 @@ fn test_bookmark_forget_fetched_bookmark() {
     ");
 
     // TEST 2: No export/import (otherwise the same as test 1)
-    test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "feature1"]);
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "forget", "--include-remotes", "feature1"],
+    );
     insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
     // Fetch works even without the export-import
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]);
@@ -810,7 +814,7 @@ fn test_bookmark_forget_fetched_bookmark() {
       @origin: qomsplrm ebeb70d8 message
     ");
 
-    // TEST 3: fetch bookmark that was moved & forgotten
+    // TEST 3: fetch bookmark that was moved & forgotten with --include-remotes
 
     // Move the bookmark in the git repo.
     git::write_commit(
@@ -820,11 +824,15 @@ fn test_bookmark_forget_fetched_bookmark() {
         "another message",
         &[first_git_repo_commit],
     );
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "feature1"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "forget", "--include-remotes", "feature1"],
+    );
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 1 bookmarks.
-    "###);
+    insta::assert_snapshot!(stderr, @r#"
+    Forgot 1 local bookmarks.
+    Forgot 1 remote bookmarks.
+    "#);
 
     // Fetching a moved bookmark does not create a conflict
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]);
@@ -836,6 +844,15 @@ fn test_bookmark_forget_fetched_bookmark() {
     feature1: tyvxnvqr 9175cb32 (empty) another message
       @origin: tyvxnvqr 9175cb32 (empty) another message
     ");
+
+    // TEST 4: If `--include-remotes` isn't used, remote bookmarks are untracked
+    test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "feature1"]);
+    insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"feature1@origin: tyvxnvqr 9175cb32 (empty) another message");
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]);
+    insta::assert_snapshot!(stdout, @"");
+    // There should be no output here since the remote bookmark wasn't forgotten
+    insta::assert_snapshot!(stderr, @"Nothing changed.");
+    insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"feature1@origin: tyvxnvqr 9175cb32 (empty) another message");
 }
 
 #[test]
@@ -876,7 +893,10 @@ fn test_bookmark_forget_deleted_or_nonexistent_bookmark() {
     // ============ End of test setup ============
 
     // We can forget a deleted bookmark
-    test_env.jj_cmd_ok(&repo_path, &["bookmark", "forget", "feature1"]);
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "forget", "--include-remotes", "feature1"],
+    );
     insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @"");
 
     // Can't forget a non-existent bookmark

cli/tests/test_git_clone.rs

@@ -793,16 +793,20 @@ fn test_git_clone_trunk_deleted(subprocess: bool) {
     "#);
     }
 
-    let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["bookmark", "forget", "main"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &clone_path,
+        &["bookmark", "forget", "--include-remotes", "main"],
+    );
     insta::allow_duplicates! {
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
-    insta::assert_snapshot!(stderr, @r"
-    Forgot 1 bookmarks.
+    insta::assert_snapshot!(stderr, @r#"
+    Forgot 1 local bookmarks.
+    Forgot 1 remote bookmarks.
     Warning: Failed to resolve `revset-aliases.trunk()`: Revision `main@origin` doesn't exist
     Hint: Use `jj config edit --repo` to adjust the `trunk()` alias.
-    ");
+    "#);
     }
 
     let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["log"]);

cli/tests/test_git_colocated.rs

@@ -384,11 +384,15 @@ fn test_git_colocated_bookmark_forget() {
       @git: rlvkpnrz 65b6b74e (empty) (no description set)
     "###);
 
-    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "foo"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &["bookmark", "forget", "--include-remotes", "foo"],
+    );
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
-    Forgot 1 bookmarks.
-    "###);
+    insta::assert_snapshot!(stderr, @r#"
+    Forgot 1 local bookmarks.
+    Forgot 1 remote bookmarks.
+    "#);
     // A forgotten bookmark is deleted in the git repo. For a detailed demo
     // explaining this, see `test_bookmark_forget_export` in
     // `test_bookmark_command.rs`.

cli/tests/test_git_fetch.rs

@@ -1590,7 +1590,10 @@ fn test_git_fetch_removed_bookmark(subprocess: bool) {
     }
 
     // Remove a2 bookmark in origin
-    test_env.jj_cmd_ok(&source_git_repo_path, &["bookmark", "forget", "a2"]);
+    test_env.jj_cmd_ok(
+        &source_git_repo_path,
+        &["bookmark", "forget", "--include-remotes", "a2"],
+    );
 
     // Fetch bookmark a1 from origin and check that a2 is still there
     let (stdout, stderr) =
@@ -1708,7 +1711,10 @@ fn test_git_fetch_removed_parent_bookmark(subprocess: bool) {
     }
 
     // Remove all bookmarks in origin.
-    test_env.jj_cmd_ok(&source_git_repo_path, &["bookmark", "forget", "glob:*"]);
+    test_env.jj_cmd_ok(
+        &source_git_repo_path,
+        &["bookmark", "forget", "--include-remotes", "glob:*"],
+    );
 
     // Fetch bookmarks master, trunk1 and a1 from origin and check that only those
     // bookmarks have been removed and that others were not rebased because of

cli/tests/test_git_push.rs

@@ -542,7 +542,10 @@ fn test_git_push_creation_unexpectedly_already_exists(subprocess: bool) {
     }
 
     // Forget bookmark1 locally
-    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "bookmark1"]);
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &["bookmark", "forget", "--include-remotes", "bookmark1"],
+    );
 
     // Create a new branh1
     test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-m=new bookmark1"]);