Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: hint for conflicted branches and change ids #2244

Merged
merged 3 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ use jj_lib::repo::{
};
use jj_lib::repo_path::{FsPathParseError, RepoPath};
use jj_lib::revset::{
DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression,
RevsetIteratorExt, RevsetParseContext, RevsetParseError, RevsetParseErrorKind,
RevsetResolutionError, RevsetWorkspaceContext,
DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetCommitRef, RevsetEvaluationError,
RevsetExpression, RevsetIteratorExt, RevsetParseContext, RevsetParseError,
RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext,
};
use jj_lib::settings::{ConfigResultExt as _, UserSettings};
use jj_lib::transaction::Transaction;
Expand Down Expand Up @@ -1006,7 +1006,7 @@ impl WorkspaceCommandHelper {
ui: &mut Ui,
) -> Result<Commit, CommandError> {
let revset_expression = self.parse_revset(revision_str, Some(ui))?;
let revset = self.evaluate_revset(revset_expression)?;
let revset = self.evaluate_revset(revset_expression.clone())?;
let mut iter = revset.iter().commits(self.repo().store()).fuse();
match (iter.next(), iter.next()) {
(Some(commit), None) => Ok(commit?),
Expand All @@ -1017,15 +1017,34 @@ impl WorkspaceCommandHelper {
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();
let hint = format!(
r#"The revset "{revision_str}" resolved to these revisions:{eol}{commits}{ellipsis}"#,
eol = "\n",
commits = commits
.iter()
.map(|c| self.format_commit_summary(c))
.join("\n"),
ellipsis = elided.then_some("\n...").unwrap_or_default()
);
let commits_summary = commits
.iter()
.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() {
// Separate hint if there's commits with same change id
wmrmrx marked this conversation as resolved.
Show resolved Hide resolved
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>`."#,
)
} else if let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(branch_name)) =
revset_expression.as_ref()
{
// Separate hint if there's a conflicted branch
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>`."#,
)
} else {
format!(
r#"The revset "{revision_str}" resolved to these revisions:
{commits_summary}"#,
)
};
Err(user_error_with_hint(
format!(r#"Revset "{revision_str}" resolved to more than one revision"#),
hint,
Expand Down
57 changes: 57 additions & 0 deletions cli/tests/test_checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,63 @@ fn test_checkout_not_single_rev() {
"###);
}

#[test]
fn test_checkout_conflicting_branches() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_success(&repo_path, &["describe", "-m", "one"]);
test_env.jj_cmd_success(&repo_path, &["new", "-m", "two", "@-"]);
test_env.jj_cmd_success(&repo_path, &["branch", "create", "foo"]);
test_env.jj_cmd_success(
&repo_path,
&[
"--at-op=@-",
"branch",
"create",
"foo",
"-r",
r#"description("one")"#,
],
);
wmrmrx marked this conversation as resolved.
Show resolved Hide resolved

// Trigger resolution of concurrent operations
test_env.jj_cmd_success(&repo_path, &["st"]);

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "foo"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "foo" resolved to more than one revision
Hint: Branch foo resolved to multiple revisions because it's conflicted.
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>`.
"###);
}

#[test]
fn test_checkout_conflicting_change_ids() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_success(&repo_path, &["describe", "-m", "one"]);
test_env.jj_cmd_success(&repo_path, &["--at-op=@-", "describe", "-m", "two"]);

// Trigger resolution of concurrent operations
test_env.jj_cmd_success(&repo_path, &["st"]);

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "qpvuntsm"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "qpvuntsm" resolved to more than one revision
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>`.
"###);
}

fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
let template = r#"commit_id ++ " " ++ description"#;
test_env.jj_cmd_success(cwd, &["log", "-T", template])
Expand Down