Skip to content

Commit

Permalink
cli: split single revset resolution hint to paragraphs
Browse files Browse the repository at this point in the history
Perhaps, this makes it slightly easier to spot the last "Hint:" line. We can
also render "Error:" and "Hint:" prefixes in different color/style.
  • Loading branch information
yuja committed Mar 24, 2024
1 parent b77a110 commit e50d96e
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 23 deletions.
40 changes: 22 additions & 18 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 <REVISION>`."#,
)
{commits_summary}"#
));
cmd_err.add_hint(
"Some of these commits have the same change id. Abandon one of them with \
`jj abandon -r <REVISION>`.",
);
} 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 <REVISION>`."#,
)
{commits_summary}"#));
cmd_err.add_hint(format!(
"Set which revision the branch points to with `jj branch set \
{branch_name} -r <REVISION>`.",
));
} 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)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/test_new_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <REVISION>`.
Hint: Set which revision the branch points to with `jj branch set foo -r <REVISION>`.
"###);
}

Expand All @@ -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 <REVISION>`.
Hint: Some of these commits have the same change id. Abandon one of them with `jj abandon -r <REVISION>`.
"###);
}

Expand Down
6 changes: 3 additions & 3 deletions cli/tests/test_rebase_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, @"");
Expand Down Expand Up @@ -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, @"");
Expand Down Expand Up @@ -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, @"");
Expand Down

0 comments on commit e50d96e

Please sign in to comment.