From a5095c1da65d9c7cf07fe9348fad5929bd8d8b2f Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 5 Jul 2024 08:32:42 +0900 Subject: [PATCH] cli: show commit summary at end of "branch create" For the same reason as cdc0cc360162. This will help notice problems like wrong target revision. The warning for multiple branches is reorganized as a hint for "-r" option, which I think is the main purpose of this warning. Unlike "squash", we don't check if an argument can be parsed as a revset because branch name is usually a valid symbol expression. --- cli/src/commands/branch/create.rs | 22 ++++++++++++++-------- cli/tests/test_branch_command.rs | 18 +++++++++++++++--- cli/tests/test_git_colocated.rs | 2 ++ cli/tests/test_revset_output.rs | 4 +++- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/cli/src/commands/branch/create.rs b/cli/src/commands/branch/create.rs index 6bdaaed3f..b23cb666d 100644 --- a/cli/src/commands/branch/create.rs +++ b/cli/src/commands/branch/create.rs @@ -61,19 +61,25 @@ pub fn cmd_branch_create( } } - if branch_names.len() > 1 { - writeln!( - ui.warning_default(), - "Creating multiple branches: {}", - branch_names.join(", "), - )?; - } - let mut tx = workspace_command.start_transaction(); for branch_name in branch_names { tx.mut_repo() .set_local_branch_target(branch_name, RefTarget::normal(target_commit.id().clone())); } + + if let Some(mut formatter) = ui.status_formatter() { + write!( + formatter, + "Created {} branches pointing to ", + branch_names.len() + )?; + tx.write_commit_summary(formatter.as_mut(), &target_commit)?; + writeln!(formatter)?; + } + if branch_names.len() > 1 && args.revision.is_none() { + writeln!(ui.hint_default(), "Use -r to specify the target revision.")?; + } + tx.finish( ui, format!( diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index 9c5461099..29311d0d3 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -25,7 +25,8 @@ fn test_branch_multiple_names() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo", "bar"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Warning: Creating multiple branches: foo, bar + Created 2 branches pointing to qpvuntsm 230dd059 bar foo | (empty) (no description set) + Hint: Use -r to specify the target revision. "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" @ bar foo 230dd059e1b0 @@ -55,6 +56,13 @@ fn test_branch_multiple_names() { ◉ 230dd059e1b0 ◉ 000000000000 "###); + + // Hint should be omitted if -r is specified + let (_stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "-r@-", "foo", "bar"]); + insta::assert_snapshot!(stderr, @r###" + Created 2 branches pointing to qpvuntsm 230dd059 bar foo | (empty) (no description set) + "###); } #[test] @@ -66,7 +74,9 @@ fn test_branch_at_root() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "fred", "-r=root()"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Created 1 branches pointing to zzzzzzzz 00000000 fred | (empty) (no description set) + "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "export"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" @@ -191,7 +201,9 @@ fn test_branch_move() { test_env.jj_cmd_ok(&repo_path, &["branch", "untrack", "foo@origin"]); test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "foo"]); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Created 1 branches pointing to mzvwutvl 66d48752 foo | (empty) (no description set) + "###); insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" foo: mzvwutvl 66d48752 (empty) (no description set) foo@origin: qpvuntsm 1eb845f3 (empty) commit diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index cd5688436..731034e44 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -400,6 +400,7 @@ fn test_git_colocated_branch_at_root() { let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo", "-r=root()"]); insta::assert_snapshot!(stderr, @r###" + Created 1 branches pointing to zzzzzzzz 00000000 foo | (empty) (no description set) Warning: Failed to export some branches: foo: Ref cannot point to the root commit in Git "###); @@ -431,6 +432,7 @@ fn test_git_colocated_conflicting_git_refs() { insta::assert_snapshot!(stdout, @""); insta::with_settings!({filters => vec![("Failed to set: .*", "Failed to set: ...")]}, { insta::assert_snapshot!(stderr, @r###" + Created 1 branches pointing to qpvuntsm 230dd059 main main/sub | (empty) (no description set) Warning: Failed to export some branches: main/sub: Failed to set: ... Hint: Git doesn't allow a branch name that looks like a parent directory of diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index df723efbb..766f84f6e 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -521,7 +521,9 @@ fn test_all_modifier() { // Command that accepts only single revision let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "-rall:@", "x"]); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Created 1 branches pointing to qpvuntsm 230dd059 x | (empty) (no description set) + "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "set", "-rall:all()", "x"]); insta::assert_snapshot!(stderr, @r###" Error: Revset "all:all()" resolved to more than one revision