From d6613304c9fb4525ad4696d9a9e8652674fc052d Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 9 May 2024 19:45:11 +0900 Subject: [PATCH] cli: print deleted branch hints at end of "branch list" output These hints shouldn't be interleaved in the template output. The new output might look a little bit worse, but I don't think it's unacceptably bad. --- cli/src/commands/branch.rs | 41 ++++++++++++++++--------- cli/tests/test_branch_command.rs | 52 ++++++++++++++++++-------------- cli/tests/test_git_fetch.rs | 5 ++- cli/tests/test_git_push.rs | 14 ++++++--- cli/tests/test_undo.rs | 5 ++- 5 files changed, 69 insertions(+), 48 deletions(-) diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 00f4713398..e790a4cd4b 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -664,6 +664,8 @@ fn cmd_branch_list( ui.request_pager(); let mut formatter = ui.stdout_formatter(); + let mut found_deleted_local_branch = false; + let mut found_deleted_tracking_local_branch = false; let branches_to_list = view.branches().filter(|(name, target)| { branch_names_to_list .as_ref() @@ -699,24 +701,11 @@ fn cmd_branch_list( template.format(&ref_name, formatter.as_mut())?; } - // TODO: move out of the loop and render as "Hint: .." ? if local_target.is_absent() && !tracking_remote_refs.is_empty() { - let found_non_git_remote = tracking_remote_refs + found_deleted_local_branch = true; + found_deleted_tracking_local_branch |= tracking_remote_refs .iter() .any(|&(remote, _)| remote != git::REMOTE_NAME_FOR_LOCAL_GIT_REPO); - if found_non_git_remote { - writeln!( - formatter.labeled("hint"), - " (this branch will be *deleted permanently* on the remote on the next `jj \ - git push`. Use `jj branch forget` to prevent this)" - )?; - } else { - writeln!( - formatter.labeled("hint"), - " (this branch will be deleted from the underlying Git repo on the next `jj \ - git export`)" - )?; - } } if args.all_remotes { @@ -727,5 +716,27 @@ fn cmd_branch_list( } } + drop(formatter); + + // Print only one of these hints. It's not important to mention unexported + // branches, but user might wonder why deleted branches are still listed. + if found_deleted_tracking_local_branch { + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "Branches marked as deleted will be *deleted permanently* on the remote on the \ + next `jj git push`. Use `jj branch forget` to prevent this." + )?; + } + } else if found_deleted_local_branch { + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "Branches marked as deleted will be deleted from the underlying Git repo on the \ + next `jj git export`." + )?; + } + } + Ok(()) } diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index a9ae82071b..087cfbead3 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -376,13 +376,10 @@ fn test_branch_delete_glob() { @origin: qpvuntsm 6fbf398c (empty) commit foo-1 (deleted) @origin: qpvuntsm 6fbf398c (empty) commit - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) foo-3 (deleted) @origin: qpvuntsm 6fbf398c (empty) commit - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) foo-4 (deleted) @origin: qpvuntsm 6fbf398c (empty) commit - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); // Malformed glob @@ -418,9 +415,10 @@ fn test_branch_delete_export() { insta::assert_snapshot!(stdout, @r###" foo (deleted) @git: rlvkpnrz 65b6b74e (empty) (no description set) - (this branch will be deleted from the underlying Git repo on the next `jj git export`) "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be deleted from the underlying Git repo on the next `jj git export`. + "###); test_env.jj_cmd_ok(&repo_path, &["git", "export"]); insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" @@ -629,7 +627,6 @@ fn test_branch_forget_deleted_or_nonexistent_branch() { insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" feature1 (deleted) @origin: mzyxwzks 9f01a0e0 message - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); // ============ End of test setup ============ @@ -1011,26 +1008,28 @@ fn test_branch_list() { local-only: wqnwkozp 4e887f78 (empty) local-only remote-delete (deleted) @origin: mnmymoky 203e60eb (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-sync: zwtyzrop c761c7ea (empty) remote-sync remote-unsync: wqnwkozp 4e887f78 (empty) local-only @origin (ahead by 1 commits, behind by 1 commits): qpsqxpyq 38ef8af7 (empty) remote-unsync "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let (stdout, stderr) = test_env.jj_cmd_ok(&local_path, &["branch", "list", "--all-remotes"]); insta::assert_snapshot!(stdout, @r###" local-only: wqnwkozp 4e887f78 (empty) local-only remote-delete (deleted) @origin: mnmymoky 203e60eb (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-sync: zwtyzrop c761c7ea (empty) remote-sync @origin: zwtyzrop c761c7ea (empty) remote-sync remote-unsync: wqnwkozp 4e887f78 (empty) local-only @origin (ahead by 1 commits, behind by 1 commits): qpsqxpyq 38ef8af7 (empty) remote-unsync remote-untrack@origin: vmortlor 71a16b05 (empty) remote-untrack "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let template = r#" concat( @@ -1081,7 +1080,6 @@ fn test_branch_list() { tracking_present: false tracking_ahead_count: 2 tracking_behind_count: 0 - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) [remote-sync] present: true conflict: false @@ -1133,7 +1131,9 @@ fn test_branch_list() { tracking_ahead_count: tracking_behind_count: "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); } #[test] @@ -1192,12 +1192,13 @@ fn test_branch_list_filtered() { local-keep: kpqxywon c7b4c09c (empty) local-keep remote-delete (deleted) @origin: yxusvupt dad5f298 (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-keep: nlwprzpn 911e9120 (empty) remote-keep remote-rewrite: xyxluytn e31634b6 (empty) rewritten @origin (ahead by 1 commits, behind by 1 commits): xyxluytn hidden 3e9a5af6 (empty) remote-rewrite "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let query = |args: &[&str]| test_env.jj_cmd_ok(&local_path, &[&["branch", "list"], args].concat()); @@ -1262,9 +1263,10 @@ fn test_branch_list_filtered() { insta::assert_snapshot!(stdout, @r###" remote-delete (deleted) @origin: yxusvupt dad5f298 (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let (stdout, stderr) = query(&["-rbranches(remote-delete)"]); insta::assert_snapshot!(stdout, @r###" "###); @@ -1280,10 +1282,11 @@ fn test_branch_list_filtered() { local-keep: kpqxywon c7b4c09c (empty) local-keep remote-delete (deleted) @origin: yxusvupt dad5f298 (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-keep: nlwprzpn 911e9120 (empty) remote-keep "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); // Unmatched name pattern shouldn't be an error. A warning can be added later. let (stdout, stderr) = query(&["local-keep", "glob:push-*"]); @@ -1438,7 +1441,6 @@ fn test_branch_list_tracked() { @git: nmzmmopx e1da745b (empty) local-only remote-delete (deleted) @origin: mnmymoky 203e60eb (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-sync: zwtyzrop c761c7ea (empty) remote-sync @git: zwtyzrop c761c7ea (empty) remote-sync @origin: zwtyzrop c761c7ea (empty) remote-sync @@ -1451,13 +1453,14 @@ fn test_branch_list_tracked() { @git: lolpmnqw 32fa6da0 (empty) upstream-sync @upstream: lolpmnqw 32fa6da0 (empty) upstream-sync "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let (stdout, stderr) = test_env.jj_cmd_ok(&local_path, &["branch", "list", "--tracked"]); insta::assert_snapshot!(stdout, @r###" remote-delete (deleted) @origin: mnmymoky 203e60eb (empty) remote-delete - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) remote-sync: zwtyzrop c761c7ea (empty) remote-sync @origin: zwtyzrop c761c7ea (empty) remote-sync remote-unsync: nmzmmopx e1da745b (empty) local-only @@ -1467,7 +1470,9 @@ fn test_branch_list_tracked() { @upstream: lolpmnqw 32fa6da0 (empty) upstream-sync "### ); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Hint: Branches marked as deleted will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this. + "###); let (stdout, stderr) = test_env.jj_cmd_ok( &local_path, @@ -1548,5 +1553,6 @@ fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { } fn get_branch_output(test_env: &TestEnvironment, repo_path: &Path) -> String { - test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes"]) + // --quiet to suppress deleted branches hint + test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes", "--quiet"]) } diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs index fb63dd4c3f..1737ecbffe 100644 --- a/cli/tests/test_git_fetch.rs +++ b/cli/tests/test_git_fetch.rs @@ -50,7 +50,8 @@ fn add_git_remote(test_env: &TestEnvironment, repo_path: &Path, remote: &str) { } fn get_branch_output(test_env: &TestEnvironment, repo_path: &Path) -> String { - test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes"]) + // --quiet to suppress deleted branches hint + test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes", "--quiet"]) } fn create_commit(test_env: &TestEnvironment, repo_path: &Path, name: &str, parents: &[&str]) { @@ -931,7 +932,6 @@ fn test_fetch_undo_what() { insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" b (deleted) @origin: vpupmnsl hidden c7d4bdcb descr_for_b - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); // Now, let's demo restoring just the remote-tracking branch. First, let's @@ -940,7 +940,6 @@ fn test_fetch_undo_what() { insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" b (deleted) @origin: vpupmnsl hidden c7d4bdcb descr_for_b - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) newbranch: qpvuntsm 230dd059 (empty) (no description set) "###); // Restoring just the remote-tracking state will not affect `newbranch`, but diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 6d8bf2b215..68ccada4ef 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -283,11 +283,14 @@ fn test_git_push_multiple() { test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "my-branch"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); // Check the setup - let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list", "--all-remotes"]); + let stdout = test_env.jj_cmd_success( + &workspace_root, + // --quiet to suppress deleted branches hint + &["branch", "list", "--all-remotes", "--quiet"], + ); insta::assert_snapshot!(stdout, @r###" branch1 (deleted) @origin: lzmmnrxq 45a3aa29 (empty) description 1 - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) branch2: yqosqzyt 15dcdaa4 (empty) foo @origin (ahead by 1 commits, behind by 1 commits): rlzusymt 8476341e (empty) description 2 my-branch: yqosqzyt 15dcdaa4 (empty) foo @@ -837,13 +840,16 @@ fn test_git_push_tracked_vs_all() { test_env.jj_cmd_ok(&workspace_root, &["branch", "delete", "branch2"]); test_env.jj_cmd_ok(&workspace_root, &["branch", "untrack", "branch1@origin"]); test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "branch3"]); - let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list", "--all-remotes"]); + let stdout = test_env.jj_cmd_success( + &workspace_root, + // --quiet to suppress deleted branches hint + &["branch", "list", "--all-remotes", "--quiet"], + ); insta::assert_snapshot!(stdout, @r###" branch1: vruxwmqv a25f24af (empty) moved branch1 branch1@origin: lzmmnrxq 45a3aa29 (empty) description 1 branch2 (deleted) @origin: rlzusymt 8476341e (empty) description 2 - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) branch3: znkkpsqq 998d6a78 (empty) moved branch2 "###); diff --git a/cli/tests/test_undo.rs b/cli/tests/test_undo.rs index 47228fe183..111b9c5e85 100644 --- a/cli/tests/test_undo.rs +++ b/cli/tests/test_undo.rs @@ -342,7 +342,6 @@ fn test_branch_track_untrack_undo() { @origin: qpvuntsm 270721f5 (empty) commit feature2 (deleted) @origin: qpvuntsm 270721f5 (empty) commit - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); // Track/untrack can be undone so long as states can be trivially merged. @@ -362,7 +361,6 @@ fn test_branch_track_untrack_undo() { @origin: qpvuntsm 270721f5 (empty) commit feature2 (deleted) @origin: qpvuntsm 270721f5 (empty) commit - (this branch will be *deleted permanently* on the remote on the next `jj git push`. Use `jj branch forget` to prevent this) "###); test_env.jj_cmd_ok(&repo_path, &["undo"]); @@ -388,5 +386,6 @@ fn test_branch_track_untrack_undo() { } fn get_branch_output(test_env: &TestEnvironment, repo_path: &Path) -> String { - test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes"]) + // --quiet to suppress deleted branches hint + test_env.jj_cmd_success(repo_path, &["branch", "list", "--all-remotes", "--quiet"]) }