Skip to content

Commit

Permalink
cli: branch: print stats even if just one branch is updated
Browse files Browse the repository at this point in the history
We usually print stats at the end of mutable operation, and I think these
messages are useful even if N = 1. I understand that "Deleted N" (N > 1) is
unusual and the original intent of these messages was to signal possible
mistakes. However, I don't think printing N=1 stats would nullify the original
purpose.

No emptiness check is needed for delete/forget, but names can be empty in
track/untrack because of noop changes.
  • Loading branch information
yuja committed Jul 4, 2024
1 parent c256ad8 commit 034859b
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 13 deletions.
4 changes: 1 addition & 3 deletions cli/src/commands/branch/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ pub fn cmd_branch_delete(
matched_branches.iter().map(|(name, _)| name).join(", ")
),
)?;
if matched_branches.len() > 1 {
writeln!(ui.status(), "Deleted {} branches.", matched_branches.len())?;
}
writeln!(ui.status(), "Deleted {} branches.", matched_branches.len())?;
Ok(())
}
4 changes: 1 addition & 3 deletions cli/src/commands/branch/forget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ pub fn cmd_branch_forget(
matched_branches.iter().map(|(name, _)| name).join(", ")
),
)?;
if matched_branches.len() > 1 {
writeln!(ui.status(), "Forgot {} branches.", matched_branches.len())?;
}
writeln!(ui.status(), "Forgot {} branches.", matched_branches.len())?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/branch/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub fn cmd_branch_track(
ui,
format!("track remote branch {}", names.iter().join(", ")),
)?;
if names.len() > 1 {
if !names.is_empty() {
writeln!(
ui.status(),
"Started tracking {} remote branches.",
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/branch/untrack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub fn cmd_branch_untrack(
ui,
format!("untrack remote branch {}", names.iter().join(", ")),
)?;
if names.len() > 1 {
if !names.is_empty() {
writeln!(
ui.status(),
"Stopped tracking {} remote branches.",
Expand Down
18 changes: 14 additions & 4 deletions cli/tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,9 @@ fn test_branch_forget_glob() {
&["branch", "forget", "foo-4", "glob:foo-*", "glob:foo-*"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Forgot 1 branches.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar-2 230dd059e1b0
◉ 000000000000
Expand Down Expand Up @@ -529,7 +531,9 @@ fn test_branch_delete_glob() {
&["branch", "delete", "foo-4", "glob:foo-*", "glob:foo-*"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Deleted 1 branches.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar-2 foo-1@origin foo-3@origin foo-4@origin 312a98d6f27b
◉ 000000000000
Expand Down Expand Up @@ -608,7 +612,9 @@ fn test_branch_forget_export() {
insta::assert_snapshot!(stderr, @"");
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "forget", "foo"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Forgot 1 branches.
"###);
// Forgetting a branch deletes local and remote-tracking branches including
// the corresponding git-tracking branch.
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");
Expand Down Expand Up @@ -734,7 +740,9 @@ fn test_branch_forget_fetched_branch() {
.unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "forget", "feature1"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Forgot 1 branches.
"###);

// Fetching a moved branch does not create a conflict
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]);
Expand Down Expand Up @@ -1014,6 +1022,7 @@ fn test_branch_track_conflict() {
);
let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "track", "main@origin"]);
insta::assert_snapshot!(stderr, @r###"
Started tracking 1 remote branches.
main (conflicted):
+ qpvuntsm e802c4f8 (empty) b
+ qpvuntsm hidden 427890ea (empty) a
Expand Down Expand Up @@ -1129,6 +1138,7 @@ fn test_branch_track_untrack_patterns() {
Warning: Git-tracking branch cannot be untracked: feature1@git
Warning: Remote branch not tracked yet: feature2@origin
Warning: Git-tracking branch cannot be untracked: main@git
Stopped tracking 1 remote branches.
"###);
insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
feature1: omvolwpu 1336caed commit
Expand Down
4 changes: 3 additions & 1 deletion cli/tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,9 @@ fn test_git_colocated_branch_forget() {

let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["branch", "forget", "foo"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(stderr, @r###"
Forgot 1 branches.
"###);
// A forgotten branch is deleted in the git repo. For a detailed demo explaining
// this, see `test_branch_forget_export` in `test_branch_command.rs`.
insta::assert_snapshot!(get_branch_output(&test_env, &workspace_root), @"");
Expand Down

0 comments on commit 034859b

Please sign in to comment.