From 82b1493f299c50ada5822fcaff902de8a7c3efc6 Mon Sep 17 00:00:00 2001 From: Willian Mori Date: Thu, 21 Sep 2023 21:24:01 -0300 Subject: [PATCH 1/3] cli: extract commits_summary variable --- cli/src/cli_util.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index c73ac8e141..3bab0046c2 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1017,14 +1017,14 @@ 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 commits_summary = commits + .iter() + .map(|c| self.format_commit_summary(c)) + .join("\n") + + elided.then_some("\n...").unwrap_or_default(); 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() + 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"#), From ef551104ef231be5563828f5335467afb27cd790 Mon Sep 17 00:00:00 2001 From: Willian Mori Date: Sat, 9 Sep 2023 19:47:27 -0300 Subject: [PATCH 2/3] cli: hint for conflicted branches --- cli/src/cli_util.rs | 27 ++++++++++++++++++++------- cli/tests/test_checkout.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 3bab0046c2..c22adaeec2 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -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; @@ -1006,7 +1006,7 @@ impl WorkspaceCommandHelper { ui: &mut Ui, ) -> Result { 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?), @@ -1022,10 +1022,23 @@ impl WorkspaceCommandHelper { .map(|c| self.format_commit_summary(c)) .join("\n") + elided.then_some("\n...").unwrap_or_default(); - let hint = format!( - r#"The revset "{revision_str}" resolved to these revisions: + let hint = 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 `."#, + ) + } 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, diff --git a/cli/tests/test_checkout.rs b/cli/tests/test_checkout.rs index ea1111048a..a0aa817966 100644 --- a/cli/tests/test_checkout.rs +++ b/cli/tests/test_checkout.rs @@ -100,6 +100,41 @@ 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")"#, + ], + ); + + // 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 `. + "###); +} + 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]) From 2c3325183c70fd6ba49990acc4e941ce0d142d4d Mon Sep 17 00:00:00 2001 From: Willian Mori Date: Sun, 17 Sep 2023 09:41:33 -0300 Subject: [PATCH 3/3] cli: hint for same change ids --- cli/src/cli_util.rs | 12 +++++++++--- cli/tests/test_checkout.rs | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index c22adaeec2..47eaaebd26 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1022,9 +1022,15 @@ impl WorkspaceCommandHelper { .map(|c| self.format_commit_summary(c)) .join("\n") + elided.then_some("\n...").unwrap_or_default(); - let hint = if let RevsetExpression::CommitRef(RevsetCommitRef::Symbol( - branch_name, - )) = revset_expression.as_ref() + let hint = if commits[0].change_id() == commits[1].change_id() { + // Separate hint if there's commits with same change id + 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 `."#, + ) + } else if let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(branch_name)) = + revset_expression.as_ref() { // Separate hint if there's a conflicted branch format!( diff --git a/cli/tests/test_checkout.rs b/cli/tests/test_checkout.rs index a0aa817966..1777637f75 100644 --- a/cli/tests/test_checkout.rs +++ b/cli/tests/test_checkout.rs @@ -135,6 +135,28 @@ fn test_checkout_conflicting_branches() { "###); } +#[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 `. + "###); +} + 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])