From e50d96e01d885207ed0fb2ca5dc4c38bfde12b18 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 23 Mar 2024 15:15:58 +0900 Subject: [PATCH] cli: split single revset resolution hint to paragraphs Perhaps, this makes it slightly easier to spot the last "Hint:" line. We can also render "Error:" and "Hint:" prefixes in different color/style. --- cli/src/cli_util.rs | 40 ++++++++++++++++++-------------- cli/tests/test_new_command.rs | 4 ++-- cli/tests/test_rebase_command.rs | 6 ++--- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index ffb34cae5b..c7d47dff73 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -789,6 +789,9 @@ impl WorkspaceCommandHelper { r#"Revset "{revision_str}" didn't resolve to any revisions"# ))), (Some(commit0), Some(commit1)) => { + let mut cmd_err = user_error(format!( + r#"Revset "{revision_str}" resolved to more than one revision"# + )); let mut iter = [commit0, commit1].into_iter().chain(iter); let commits: Vec<_> = iter.by_ref().take(5).try_collect()?; let elided = iter.next().is_some(); @@ -797,40 +800,41 @@ impl WorkspaceCommandHelper { .map(|c| self.format_commit_summary(c)) .join("\n") + elided.then_some("\n...").unwrap_or_default(); - let hint = if commits[0].change_id() == commits[1].change_id() { + if commits[0].change_id() == commits[1].change_id() { // Separate hint if there's commits with same change id - format!( + cmd_err.add_hint(format!( r#"The revset "{revision_str}" resolved to these revisions: -{commits_summary} -Some of these commits have the same change id. Abandon one of them with `jj abandon -r `."#, - ) +{commits_summary}"# + )); + cmd_err.add_hint( + "Some of these commits have the same change id. Abandon one of them with \ + `jj abandon -r `.", + ); } else if let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(branch_name)) = revset_expression.as_ref() { // Separate hint if there's a conflicted branch - format!( + cmd_err.add_hint(format!( r#"Branch {branch_name} resolved to multiple revisions because it's conflicted. It resolved to these revisions: -{commits_summary} -Set which revision the branch points to with `jj branch set {branch_name} -r `."#, - ) +{commits_summary}"#)); + cmd_err.add_hint(format!( + "Set which revision the branch points to with `jj branch set \ + {branch_name} -r `.", + )); } else { - let mut hint = format!( + cmd_err.add_hint(format!( r#"The revset "{revision_str}" resolved to these revisions: {commits_summary}"#, - ); + )); if should_hint_about_all_prefix { - hint.push_str(&format!( - "\nPrefix the expression with 'all:' to allow any number of revisions \ + cmd_err.add_hint(format!( + "Prefix the expression with 'all:' to allow any number of revisions \ (i.e. 'all:{revision_str}')." )); } - hint }; - Err(user_error_with_hint( - format!(r#"Revset "{revision_str}" resolved to more than one revision"#), - hint, - )) + Err(cmd_err) } } } diff --git a/cli/tests/test_new_command.rs b/cli/tests/test_new_command.rs index a67752af32..cb5dec8b82 100644 --- a/cli/tests/test_new_command.rs +++ b/cli/tests/test_new_command.rs @@ -497,7 +497,7 @@ fn test_new_conflicting_branches() { It resolved to these revisions: kkmpptxz 66c6502d foo?? | (empty) two qpvuntsm a9330854 foo?? | (empty) one - Set which revision the branch points to with `jj branch set foo -r `. + Hint: Set which revision the branch points to with `jj branch set foo -r `. "###); } @@ -519,7 +519,7 @@ fn test_new_conflicting_change_ids() { Hint: The revset "qpvuntsm" resolved to these revisions: qpvuntsm?? d2ae6806 (empty) two qpvuntsm?? a9330854 (empty) one - Some of these commits have the same change id. Abandon one of them with `jj abandon -r `. + Hint: Some of these commits have the same change id. Abandon one of them with `jj abandon -r `. "###); } diff --git a/cli/tests/test_rebase_command.rs b/cli/tests/test_rebase_command.rs index a773307af4..49b062551f 100644 --- a/cli/tests/test_rebase_command.rs +++ b/cli/tests/test_rebase_command.rs @@ -169,7 +169,7 @@ fn test_rebase_branch() { Hint: The revset "e|d" resolved to these revisions: znkkpsqq e52756c8 e | e vruxwmqv 514fa6b2 d | d - Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:e|d'). + Hint: Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:e|d'). "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b=all:e|d", "-d=b"]); insta::assert_snapshot!(stdout, @""); @@ -480,7 +480,7 @@ fn test_rebase_multiple_destinations() { Hint: The revset "b|c" resolved to these revisions: royxmykx fe2e8e8b c | c zsuskuln d370aee1 b | b - Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:b|c'). + Hint: Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:b|c'). "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "a", "-d", "all:b|c"]); insta::assert_snapshot!(stdout, @""); @@ -616,7 +616,7 @@ fn test_rebase_with_descendants() { Hint: The revset "b|d" resolved to these revisions: vruxwmqv df54a9fd d | d zsuskuln d370aee1 b | b - Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:b|d'). + Hint: Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:b|d'). "###); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=all:b|d", "-d=a"]); insta::assert_snapshot!(stdout, @"");