Skip to content

Commit

Permalink
cli: print deleted branch hints at end of "branch list" output
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
yuja committed May 11, 2024
1 parent 2f3ac4e commit d661330
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 48 deletions.
41 changes: 26 additions & 15 deletions cli/src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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(())
}
52 changes: 29 additions & 23 deletions cli/tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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###"
Expand Down Expand Up @@ -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 ============
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1133,7 +1131,9 @@ fn test_branch_list() {
tracking_ahead_count: <Error: Not a tracked remote ref>
tracking_behind_count: <Error: Not a tracked remote ref>
"###);
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]
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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###"
"###);
Expand All @@ -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-*"]);
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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"])
}
5 changes: 2 additions & 3 deletions cli/tests/test_git_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 10 additions & 4 deletions cli/tests/test_git_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
"###);

Expand Down
5 changes: 2 additions & 3 deletions cli/tests/test_undo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"]);
Expand All @@ -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"])
}

0 comments on commit d661330

Please sign in to comment.