From 9ceb70d7bd33ecbc8b9c4f27752181abe2845c7f Mon Sep 17 00:00:00 2001 From: Essien Ita Essien <34972+essiene@users.noreply.github.com> Date: Thu, 25 Jul 2024 09:10:41 +0100 Subject: [PATCH] Change conflict hint depending on state of working commit. To avoid always printing the rebase instructions to fix a conflict even when a child commit to fix the conflict already exists, implement the following: * If working commit has conflicts: * if any decendants are conflict free: "Conflict fixed in ". For this, print the short_change_hash of the first decendant without conflicts. * if no decendants are conflict free: Print the existing message we print today. * If working commit has no conflicts: * If any parent has conflicts: "Conflict in parent is resolved in working copy". Explicitly not printing the "conflicting parent" here, since a merge commit could have conflict in multiple parents. * If no parent has any conflicts: exit quietly. * Rename existing unittest * Also update Changelog here --- CHANGELOG.md | 4 ++ cli/src/commands/status.rs | 65 ++++++++++++++++++++------- cli/tests/test_status_command.rs | 77 +++++++++++++++++++++++++++++++- 3 files changed, 129 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1444c566ba..c990b7075d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed bugs +* `jj status` will show different messages in a conflicted tree, depending + on the state of the working commit. In particular, if a child commit has + fixed the conflict, this will be reflected in the hint provided by `jj status` + * `jj diff --git` no longer shows the contents of binary files. * Windows binaries no longer require `vcruntime140.dll` to be installed diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 7eb3eb4f3d2..efc4fe5cf90 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -17,7 +17,7 @@ use jj_lib::repo::Repo; use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate}; use tracing::instrument; -use crate::cli_util::{print_conflicted_paths, CommandHelper}; +use crate::cli_util::{print_conflicted_paths, short_change_hash, CommandHelper}; use crate::command_error::CommandError; use crate::diff_util::DiffFormat; use crate::revset_util; @@ -91,21 +91,54 @@ pub(crate) fn cmd_status( writeln!(formatter)?; } - let wc_revset = RevsetExpression::commit(wc_commit.id().clone()); - // Ancestors with conflicts, excluding the current working copy commit. - let ancestors_conflicts = workspace_command - .attach_revset_evaluator( - wc_revset - .parents() - .ancestors() - .filtered(RevsetFilterPredicate::HasConflict) - .minus(&revset_util::parse_immutable_expression( - &workspace_command.revset_parse_context(), - )?), - )? - .evaluate_to_commit_ids()? - .collect(); - workspace_command.report_repo_conflicts(formatter, repo, ancestors_conflicts)?; + if wc_commit.has_conflict()? { + let wc_revset = RevsetExpression::commit(wc_commit.id().clone()); + + if let Some(first_descendant_no_conflict) = workspace_command + .attach_revset_evaluator( + wc_revset + .children() + .descendants() + .filtered(RevsetFilterPredicate::HasNoConflict), + )? + .evaluate_to_commits()? + .collect::>() + .pop() + { + writeln!( + formatter.labeled("conflict"), + "Conflict has been resolved in descendants: {}", + short_change_hash(first_descendant_no_conflict?.change_id()) + )?; + } else { + // Ancestors with conflicts, excluding the current working copy commit. + let ancestors_conflicts: Vec<_> = workspace_command + .attach_revset_evaluator( + wc_revset + .parents() + .ancestors() + .filtered(RevsetFilterPredicate::HasConflict) + .minus(&revset_util::parse_immutable_expression( + &workspace_command.revset_parse_context(), + )?), + )? + .evaluate_to_commit_ids()? + .collect(); + + workspace_command.report_repo_conflicts(formatter, repo, ancestors_conflicts)?; + } + } else { + for parent in wc_commit.parents() { + let parent = parent?; + if parent.has_conflict()? { + writeln!( + formatter.labeled("conflict"), + "Conflict in parent commit has been resolved in working copy" + )?; + break; + } + } + } } else { writeln!(formatter, "No working copy")?; } diff --git a/cli/tests/test_status_command.rs b/cli/tests/test_status_command.rs index 81bb9e03728..f74af197d7f 100644 --- a/cli/tests/test_status_command.rs +++ b/cli/tests/test_status_command.rs @@ -106,7 +106,7 @@ fn test_status_filtered() { // See #[test] -fn test_status_display_rebase_instructions() { +fn test_status_display_rebase_instructions_if_no_descendant_resolves_conflict() { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -177,6 +177,81 @@ fn test_status_display_rebase_instructions() { "###); } +#[test] +fn test_status_display_if_child_resolves_conflict() { + 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", "fix_conflict1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + test_env.jj_cmd_ok( + &repo_path, + &["new", "--message", "child of conflict fixing commit"], + ); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "::@"]); + + insta::assert_snapshot!(stdout, @r###" + @ vruxwmqv test.user@example.com 2001-02-03 08:05:14 c6ea5f6c + │ (empty) child of conflict fixing commit + ○ yqosqzyt test.user@example.com 2001-02-03 08:05:14 15837663 + │ fix_conflict1 + × royxmykx test.user@example.com 2001-02-03 08:05:12 a4e88714 conflict + │ (empty) boom-cont + × mzvwutvl test.user@example.com 2001-02-03 08:05:11 538415e7 conflict + ├─╮ (empty) boom + │ ○ kkmpptxz test.user@example.com 2001-02-03 08:05:10 1e8c2956 + │ │ First part of conflicting change + ○ │ zsuskuln test.user@example.com 2001-02-03 08:05:11 2c8b19fd + ├─╯ Second part of conflicting change + ○ qpvuntsm test.user@example.com 2001-02-03 08:05:08 aade7195 + │ 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 65143fef (conflict) (empty) boom-cont-2 + Parent commit: royxmykx a4e88714 (conflict) (empty) boom-cont + Conflict has been resolved in descendant: foobar + "###); +} + #[test] fn test_status_simplify_conflict_sides() { let test_env = TestEnvironment::default();