From a4a148104cac7d63b0b476191bce086f7e7c13a8 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Mon, 6 Jun 2022 15:50:45 -0700
Subject: [PATCH] cli: don't say "Creating branch push-*" if it already exists

I was a bit surprised to see the message when I used `jj git push
--change @-` on a commit that already had a branch because I had
pushed it earlier.

The fix means that we instead print the message even if we later
abandon the transaction (so the branch-creation is not persisted)
because the commit is open, for example. That's already what happens
if the commit is missing a description, and since we're planning to
remove the open/closed concept, I don't think this patch makes it much
worse. We probably should improve it later by printing the message
only once the push has succeeded.
---
 src/commands.rs        | 12 +++++++-----
 tests/test_git_push.rs | 15 +++++++++++----
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index 33bda8856..0ec28acdc 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -4928,16 +4928,18 @@ fn cmd_git_push(
             ui.settings().push_branch_prefix(),
             commit.change_id().hex()
         );
-        tx.mut_repo()
-            .set_local_branch(branch_name.clone(), RefTarget::Normal(commit.id().clone()));
-        if let Some(update) =
-            branch_updates_for_push(tx.mut_repo().as_repo_ref(), &args.remote, &branch_name)?
-        {
+        if tx.mut_repo().get_local_branch(&branch_name).is_none() {
             writeln!(
                 ui,
                 "Creating branch {} for revision {}",
                 branch_name, change_str
             )?;
+        }
+        tx.mut_repo()
+            .set_local_branch(branch_name.clone(), RefTarget::Normal(commit.id().clone()));
+        if let Some(update) =
+            branch_updates_for_push(tx.mut_repo().as_repo_ref(), &args.remote, &branch_name)?
+        {
             branch_updates.insert(branch_name.clone(), update);
         } else {
             writeln!(
diff --git a/tests/test_git_push.rs b/tests/test_git_push.rs
index a0a92b370..0ebfa1297 100644
--- a/tests/test_git_push.rs
+++ b/tests/test_git_push.rs
@@ -14,7 +14,9 @@
 
 use std::path::PathBuf;
 
-use crate::common::TestEnvironment;
+use regex::Regex;
+
+use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
 
 pub mod common;
 
@@ -60,9 +62,14 @@ fn test_git_push_open() {
     Error: Won't push open commit
     "###);
     // When pushing with `--change`, won't push if it points to an open commit
-    let stderr =
-        test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--change", "my-branch"]);
-    insta::assert_snapshot!(stderr, @r###"
+    let assert = test_env
+        .jj_cmd(&workspace_root, &["git", "push", "--change", "my-branch"])
+        .assert();
+    let branch_pattern = Regex::new("push-[0-9a-f]+").unwrap();
+    insta::assert_snapshot!(branch_pattern.replace(&get_stdout_string(&assert), "<branch>"), @r###"
+    Creating branch <branch> for revision my-branch
+    "###);
+    insta::assert_snapshot!(get_stderr_string(&assert), @r###"
     Error: Won't push open commit
     "###);
 }