Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: show commit summary at end of "branch move" #4029

Merged
merged 2 commits into from
Jul 4, 2024
Merged

cli: show commit summary at end of "branch move" #4029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4422a06
cli: show commit summary at end of "branch move"
yuja Jul 4, 2024
61808af
cli: rephrase hint for updating just one branch by "branch move"
yuja Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions cli/src/commands/branch/move.rs
View file
Open in desktop
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,16 @@ 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 just one of the branches."
)?;
}
Ok(())
}
19 changes: 12 additions & 7 deletions cli/tests/test_branch_command.rs
View file
Open in desktop
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,14 +247,14 @@ 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
Hint: Specify branch by name to update one.
Moved 2 branches to vruxwmqv a2781dd9 b1 c1 | (empty) head2
Hint: Specify branch by name to update just one of the branches.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ b1 c1 a2781dd9ee37
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