From 7a0b032519b5fe5b4ddb3bfea8cfee6bdcfe3c78 Mon Sep 17 00:00:00 2001 From: "Alexis (Poliorcetics) Bourget" Date: Tue, 9 Apr 2024 22:21:29 +0200 Subject: [PATCH] cli: status: when current change has conflicts, display instructions to resolve them --- cli/src/cli_util.rs | 76 +++++++++++++++++++------------- cli/src/commands/status.rs | 11 +++++ cli/tests/test_status_command.rs | 73 ++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 30 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 924413d189..ded3bc8181 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1331,47 +1331,63 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin "There are still unresolved conflicts in rebased descendants.", )?; } - let root_conflicts_revset = RevsetExpression::commits( + + self.report_repo_conflicts( + fmt.as_mut(), + new_repo, added_conflict_commits .iter() .map(|commit| commit.id().clone()) .collect(), - ) + )?; + } + + Ok(()) + } + + pub fn report_repo_conflicts( + &self, + fmt: &mut dyn Formatter, + repo: &ReadonlyRepo, + conflicted_commits: Vec, + ) -> Result<(), CommandError> { + let only_one_conflicted_commit = conflicted_commits.len() == 1; + let root_conflicts_revset = RevsetExpression::commits(conflicted_commits) .roots() - .evaluate_programmatic(new_repo)?; + .evaluate_programmatic(repo)?; - let root_conflict_commits: Vec<_> = root_conflicts_revset - .iter() - .commits(new_repo.store()) - .try_collect()?; - if !root_conflict_commits.is_empty() { - fmt.push_label("hint")?; - if added_conflict_commits.len() == 1 { - writeln!(fmt, "To resolve the conflicts, start by updating to it:",)?; - } else if root_conflict_commits.len() == 1 { - writeln!( - fmt, - "To resolve the conflicts, start by updating to the first one:", - )?; - } else { - writeln!( - fmt, - "To resolve the conflicts, start by updating to one of the first ones:", - )?; - } - for commit in root_conflict_commits { - writeln!(fmt, " jj new {}", short_change_hash(commit.change_id()))?; - } + let root_conflict_change_ids: Vec<_> = root_conflicts_revset + .iter() + .commits(repo.store()) + .map(|maybe_commit| maybe_commit.map(|c| c.change_id().clone())) + .try_collect()?; + + if !root_conflict_change_ids.is_empty() { + fmt.push_label("hint")?; + if only_one_conflicted_commit { + writeln!(fmt, "To resolve the conflicts, start by updating to it:",)?; + } else if root_conflict_change_ids.len() == 1 { writeln!( fmt, - r#"Then use `jj resolve`, or edit the conflict markers in the file directly. -Once the conflicts are resolved, you may want inspect the result with `jj diff`. -Then run `jj squash` to move the resolution into the conflicted commit."#, + "To resolve the conflicts, start by updating to the first one:", + )?; + } else { + writeln!( + fmt, + "To resolve the conflicts, start by updating to one of the first ones:", )?; - fmt.pop_label()?; } + for change_id in root_conflict_change_ids { + writeln!(fmt, " jj new {}", short_change_hash(&change_id))?; + } + writeln!( + fmt, + r#"Then use `jj resolve`, or edit the conflict markers in the file directly. +Once the conflicts are resolved, you may want inspect the result with `jj diff`. +Then run `jj squash` to move the resolution into the conflicted commit."#, + )?; + fmt.pop_label()?; } - Ok(()) } } diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 6daf7d0ba3..0030857922 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -14,6 +14,7 @@ use itertools::Itertools; use jj_lib::repo::Repo; +use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate}; use jj_lib::rewrite::merge_commit_trees; use tracing::instrument; @@ -91,6 +92,16 @@ pub(crate) fn cmd_status( template.format(&parent, formatter)?; writeln!(formatter)?; } + + let wc_revset = RevsetExpression::commit(wc_commit.id().clone()); + // Ancestors with conflicts, excluding the current working copy commit. + let ancestors_conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict) + .intersection(&wc_revset.ancestors()) + .minus(&wc_revset) + .evaluate_programmatic(repo.as_ref())? + .iter() + .collect(); + workspace_command.report_repo_conflicts(formatter, repo, ancestors_conflicts)?; } else { writeln!(formatter, "No working copy")?; } diff --git a/cli/tests/test_status_command.rs b/cli/tests/test_status_command.rs index 3bf22d6144..80b7b50fcf 100644 --- a/cli/tests/test_status_command.rs +++ b/cli/tests/test_status_command.rs @@ -81,3 +81,76 @@ fn test_status_filtered() { Parent commit: zzzzzzzz 00000000 (empty) (no description set) "###); } + +// See +#[test] +fn test_status_display_rebase_instructions() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + + let repo_path = test_env.env_root().join("repo"); + let conflicted_path = repo_path.join("conflicted.txt"); + + // PARENT: Write the initial file + std::fs::write(&conflicted_path, "initial contents").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["describe", "--message", "Initial contents"]); + + // CHILD1: New commit on top of + test_env.jj_cmd_ok( + &repo_path, + &["new", "--message", "First part of conflicting change"], + ); + std::fs::write(&conflicted_path, "Child 1").unwrap(); + + // CHILD2: New commit also on top of + test_env.jj_cmd_ok( + &repo_path, + &[ + "new", + "--message", + "Second part of conflicting change", + "@-", + ], + ); + std::fs::write(&conflicted_path, "Child 2").unwrap(); + + // CONFLICT: New commit that is conflicted by merging and + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom", "all:(@-)+"]); + // Adding more descendants to ensure we correctly find the root ancestors with + // conflicts, not just the parents. + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont"]); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont-2"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "::@"]); + + insta::assert_snapshot!(stdout, @r###" + @ yqosqzyt test.user@example.com 2001-02-03 08:05:13 d41fb742 conflict + │ (empty) boom-cont-2 + ◉ royxmykx test.user@example.com 2001-02-03 08:05:12 c2389cec conflict + │ (empty) boom-cont + ◉ mzvwutvl test.user@example.com 2001-02-03 08:05:11 c57f2050 conflict + ├─╮ (empty) boom + │ ◉ kkmpptxz test.user@example.com 2001-02-03 08:05:10 55ce6709 + │ │ First part of conflicting change + ◉ │ zsuskuln test.user@example.com 2001-02-03 08:05:11 ba5f8773 + ├─╯ Second part of conflicting change + ◉ qpvuntsm test.user@example.com 2001-02-03 08:05:08 98e0dcf8 + │ Initial contents + ◉ zzzzzzzz root() 00000000 + "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["status"]); + + insta::assert_snapshot!(stdout, @r###" + The working copy is clean + There are unresolved conflicts at these paths: + conflicted.txt 2-sided conflict + Working copy : yqosqzyt d41fb742 (conflict) (empty) boom-cont-2 + Parent commit: royxmykx c2389cec (conflict) (empty) boom-cont + To resolve the conflicts, start by updating to the first one: + jj new mzvwutvlkqwt + Then use `jj resolve`, or edit the conflict markers in the file directly. + Once the conflicts are resolved, you may want inspect the result with `jj diff`. + Then run `jj squash` to move the resolution into the conflicted commit. + "###); +}