From 034859b52fb8abd678944725427909ccd932e4e1 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 21 Jun 2024 17:41:53 +0900 Subject: [PATCH] cli: branch: print stats even if just one branch is updated 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. --- cli/src/commands/branch/delete.rs | 4 +--- cli/src/commands/branch/forget.rs | 4 +--- cli/src/commands/branch/track.rs | 2 +- cli/src/commands/branch/untrack.rs | 2 +- cli/tests/test_branch_command.rs | 18 ++++++++++++++---- cli/tests/test_git_colocated.rs | 4 +++- 6 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cli/src/commands/branch/delete.rs b/cli/src/commands/branch/delete.rs index c9d5700766..ea3588cae7 100644 --- a/cli/src/commands/branch/delete.rs +++ b/cli/src/commands/branch/delete.rs @@ -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(()) } diff --git a/cli/src/commands/branch/forget.rs b/cli/src/commands/branch/forget.rs index a0b3b05a56..65a0dd7485 100644 --- a/cli/src/commands/branch/forget.rs +++ b/cli/src/commands/branch/forget.rs @@ -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(()) } diff --git a/cli/src/commands/branch/track.rs b/cli/src/commands/branch/track.rs index 5dab3a29d0..b7ab4daf5c 100644 --- a/cli/src/commands/branch/track.rs +++ b/cli/src/commands/branch/track.rs @@ -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.", diff --git a/cli/src/commands/branch/untrack.rs b/cli/src/commands/branch/untrack.rs index 498ecfe29b..1b68200c56 100644 --- a/cli/src/commands/branch/untrack.rs +++ b/cli/src/commands/branch/untrack.rs @@ -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.", diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index 2c444824b0..4ca8b0fb06 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -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 @@ -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 @@ -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), @""); @@ -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"]); @@ -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 @@ -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 diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index f443fef55b..82bf8998df 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -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), @"");