From d43704c978564fae55e50c30bdd79994a80f81d6 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Fri, 20 Oct 2023 13:14:09 +0900
Subject: [PATCH] cli: do not error out on re-track/untrack, show warning
 instead

If we add glob support, users will probably want to do something like
'jj branch untrack glob:"*@origin"'. It would be annoying if the command
failed just because one of the remote branches has already been untracked.
Since branch tracking/untracking is idempotent, it's safe to continue in
those cases.
---
 cli/src/commands/branch.rs       | 27 ++++++++++++++++-----------
 cli/tests/test_branch_command.rs | 29 ++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs
index 0e5349d64..a2a8b903c 100644
--- a/cli/src/commands/branch.rs
+++ b/cli/src/commands/branch.rs
@@ -402,18 +402,21 @@ fn cmd_branch_track(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let view = workspace_command.repo().view();
+    let mut names = Vec::new();
     for name in &args.names {
         let remote_ref = view.get_remote_branch(&name.branch, &name.remote);
         if remote_ref.is_absent() {
             return Err(user_error(format!("No such remote branch: {name}")));
         }
         if remote_ref.is_tracking() {
-            return Err(user_error(format!("Remote branch already tracked: {name}")));
+            writeln!(ui.warning(), "Remote branch already tracked: {name}")?;
+        } else {
+            names.push(name.clone());
         }
     }
     let mut tx = workspace_command
-        .start_transaction(&format!("track remote {}", make_branch_term(&args.names)));
-    for name in &args.names {
+        .start_transaction(&format!("track remote {}", make_branch_term(&names)));
+    for name in &names {
         tx.mut_repo()
             .track_remote_branch(&name.branch, &name.remote);
     }
@@ -428,22 +431,24 @@ fn cmd_branch_untrack(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let view = workspace_command.repo().view();
+    let mut names = Vec::new();
     for name in &args.names {
-        if name.remote == git::REMOTE_NAME_FOR_LOCAL_GIT_REPO {
-            // This restriction can be lifted if we want to support untracked @git branches.
-            return Err(user_error("Git-tracking branch cannot be untracked"));
-        }
         let remote_ref = view.get_remote_branch(&name.branch, &name.remote);
         if remote_ref.is_absent() {
             return Err(user_error(format!("No such remote branch: {name}")));
         }
-        if !remote_ref.is_tracking() {
-            return Err(user_error(format!("Remote branch not tracked yet: {name}")));
+        if name.remote == git::REMOTE_NAME_FOR_LOCAL_GIT_REPO {
+            // This restriction can be lifted if we want to support untracked @git branches.
+            writeln!(ui.warning(), "Git-tracking branch cannot be untracked: {name}")?;
+        } else if !remote_ref.is_tracking() {
+            writeln!(ui.warning(), "Remote branch not tracked yet: {name}")?;
+        } else {
+            names.push(name.clone());
         }
     }
     let mut tx = workspace_command
-        .start_transaction(&format!("untrack remote {}", make_branch_term(&args.names)));
-    for name in &args.names {
+        .start_transaction(&format!("untrack remote {}", make_branch_term(&names)));
+    for name in &names {
         tx.mut_repo()
             .untrack_remote_branch(&name.branch, &name.remote);
     }
diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs
index 7b1f3693f..1a9b884c3 100644
--- a/cli/tests/test_branch_command.rs
+++ b/cli/tests/test_branch_command.rs
@@ -720,22 +720,33 @@ fn test_branch_track_untrack_bad_branches() {
 
     // Track already tracked branch
     test_env.jj_cmd_ok(&repo_path, &["branch", "track", "feature1@origin"]);
-    insta::assert_snapshot!(
-        test_env.jj_cmd_failure(&repo_path, &["branch", "track", "feature1@origin"]), @r###"
-    Error: Remote branch already tracked: feature1@origin
+    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "track", "feature1@origin"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Remote branch already tracked: feature1@origin
+    Nothing changed.
     "###);
 
     // Untrack non-tracking branch
-    insta::assert_snapshot!(
-        test_env.jj_cmd_failure(&repo_path, &["branch", "untrack", "feature2@origin"]), @r###"
-    Error: Remote branch not tracked yet: feature2@origin
+    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "untrack", "feature2@origin"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Remote branch not tracked yet: feature2@origin
+    Nothing changed.
     "###);
 
     // Untrack Git-tracking branch
     test_env.jj_cmd_ok(&repo_path, &["git", "export"]);
-    insta::assert_snapshot!(
-        test_env.jj_cmd_failure(&repo_path, &["branch", "untrack", "main@git"]), @r###"
-    Error: Git-tracking branch cannot be untracked
+    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "untrack", "main@git"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Git-tracking branch cannot be untracked: main@git
+    Nothing changed.
+    "###);
+    insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
+    feature1: omvolwpu 1336caed commit
+      @git: omvolwpu 1336caed commit
+      @origin: omvolwpu 1336caed commit
+    feature2@origin: omvolwpu 1336caed commit
+    main: qpvuntsm 230dd059 (empty) (no description set)
+      @git: qpvuntsm 230dd059 (empty) (no description set)
     "###);
 }