Skip to content

Commit

Permalink
cli: show commit summary at end of "branch move"
Browse files Browse the repository at this point in the history
It's nice to see the result of "branch move", "create", etc., and this is more
important in "branch move" because the source branches can be specified in an
abstracted way. I originally considered printing a list of affected branches,
but it looked rather verbose. Since the destination revision is unique, we can
use commit_summary template instead.

This patch also removes a warning about multiple branches because the branch
names are included in the commit summary. I think the hint message is good
enough to signal possible mistake.
  • Loading branch information
yuja committed Jul 4, 2024
1 parent 0c0e001 commit cdc0cc3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
27 changes: 14 additions & 13 deletions cli/src/commands/branch/move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub fn cmd_branch_move(
let mut workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo().clone();

let target_commit = workspace_command.resolve_single_rev(&args.to)?;
let matched_branches = {
let is_source_commit = if !args.from.is_empty() {
workspace_command
Expand All @@ -75,7 +76,7 @@ pub fn cmd_branch_move(
} else {
Box::new(|_: &CommitId| true)
};
if !args.names.is_empty() {
let mut branches = if !args.names.is_empty() {
find_branches_with(&args.names, |pattern| {
repo.view()
.local_branches_matching(pattern)
Expand All @@ -86,24 +87,16 @@ pub fn cmd_branch_move(
.local_branches()
.filter(|(_, target)| target.added_ids().any(&is_source_commit))
.collect()
}
};
// Noop matches aren't error, but should be excluded from stats.
branches.retain(|(_, old_target)| old_target.as_normal() != Some(target_commit.id()));
branches
};
let target_commit = workspace_command.resolve_single_rev(&args.to)?;

if matched_branches.is_empty() {
writeln!(ui.status(), "No branches to update.")?;
return Ok(());
}
if matched_branches.len() > 1 {
writeln!(
ui.warning_default(),
"Updating multiple branches: {}",
matched_branches.iter().map(|(name, _)| name).join(", "),
)?;
if args.names.is_empty() {
writeln!(ui.hint_default(), "Specify branch by name to update one.")?;
}
}

if !args.allow_backwards {
if let Some((name, _)) = matched_branches
Expand Down Expand Up @@ -131,5 +124,13 @@ pub fn cmd_branch_move(
),
)?;

if let Some(mut formatter) = ui.status_formatter() {
write!(formatter, "Moved {} branches to ", matched_branches.len())?;
workspace_command.write_commit_summary(formatter.as_mut(), &target_commit)?;
writeln!(formatter)?;
}
if matched_branches.len() > 1 && args.names.is_empty() {
writeln!(ui.hint_default(), "Specify branch by name to update one.")?;
}
Ok(())
}
17 changes: 11 additions & 6 deletions cli/tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ fn test_branch_move() {
insta::assert_snapshot!(stderr, @"");

let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "foo"]);
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Moved 1 branches to mzvwutvl 167f90e7 foo | (empty) (no description set)
"###);

let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "--to=@-", "foo"]);
insta::assert_snapshot!(stderr, @r###"
Expand All @@ -157,7 +159,9 @@ fn test_branch_move() {
&repo_path,
&["branch", "move", "--to=@-", "--allow-backwards", "foo"],
);
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Moved 1 branches to qpvuntsm 230dd059 foo | (empty) (no description set)
"###);

// Delete branch locally, but is still tracking remote
test_env.jj_cmd_ok(&repo_path, &["describe", "@-", "-mcommit"]);
Expand Down Expand Up @@ -243,13 +247,13 @@ fn test_branch_move_matching() {
// Noop move
let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "--to=a1", "a2"]);
insta::assert_snapshot!(stderr, @r###"
Nothing changed.
No branches to update.
"###);

// Move from multiple revisions
let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "move", "--from=::@"]);
insta::assert_snapshot!(stderr, @r###"
Warning: Updating multiple branches: b1, c1
Moved 2 branches to vruxwmqv a2781dd9 b1 c1 | (empty) head2
Hint: Specify branch by name to update one.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
Expand All @@ -266,7 +270,6 @@ fn test_branch_move_matching() {
// Try to move multiple branches, but one of them isn't fast-forward
let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "move", "glob:?1"]);
insta::assert_snapshot!(stderr, @r###"
Warning: Updating multiple branches: a1, b1, c1
Error: Refusing to move branch backwards or sideways: a1
Hint: Use --allow-backwards to allow it.
"###);
Expand All @@ -285,7 +288,9 @@ fn test_branch_move_matching() {
&repo_path,
&["branch", "move", "--from=::a1+", "--to=a1+", "glob:?1"],
);
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Moved 1 branches to kkmpptxz 6b5e840e a1 | (empty) head1
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ a2781dd9ee37
◉ c1 f4f38657a3dd
Expand Down

0 comments on commit cdc0cc3

Please sign in to comment.