From 5efaa633c41026384860aaff5adc77ccf799b158 Mon Sep 17 00:00:00 2001 From: "Alexis (Poliorcetics) Bourget" Date: Sat, 30 Mar 2024 18:34:51 +0100 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 | 55 +++++++++++++++++++++++ 3 files changed, 112 insertions(+), 30 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 1c37c99fbb..96a62c87d1 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1319,47 +1319,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 982570843a..7ad66c109d 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -15,6 +15,7 @@ use itertools::Itertools; use jj_lib::matchers::EverythingMatcher; use jj_lib::repo::Repo; +use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate}; use jj_lib::rewrite::merge_commit_trees; use tracing::instrument; @@ -84,6 +85,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 574dda0712..03f72e245e 100644 --- a/cli/tests/test_status_command.rs +++ b/cli/tests/test_status_command.rs @@ -62,3 +62,58 @@ fn test_status_ignored_gitignore() { 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, &["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. + "###); +}