From 6428b9ef75877ce89e91ae13ade9ab63ebae7bc6 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 | 291 ++++++++++++++++++++++++++++++- 3 files changed, 342 insertions(+), 18 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..50af98e6e1e 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"]); @@ -143,7 +143,7 @@ fn test_status_display_rebase_instructions() { 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", "::@"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()::"]); insta::assert_snapshot!(stdout, @r###" @ yqosqzyt test.user@example.com 2001-02-03 08:05:13 65143fef conflict @@ -177,6 +177,293 @@ fn test_status_display_rebase_instructions() { "###); } +#[test] +fn test_status_working_commit_noconflict_parent_noconflict() { + 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:(@-)+"]); + + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont"]); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 2"]); + std::fs::write(&conflicted_path, "edit not conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["status"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()::"]); + + insta::assert_snapshot!(stdout, @r###" + @ vruxwmqv test.user@example.com 2001-02-03 08:05:15 376a25f2 + │ fixed 2 + ○ yqosqzyt test.user@example.com 2001-02-03 08:05:14 f10a05bb + │ fixed 1 + × 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###" + Working copy changes: + M conflicted.txt + Working copy : vruxwmqv 376a25f2 fixed 2 + Parent commit: yqosqzyt f10a05bb fixed 1 + "###); +} + +#[test] +fn test_status_working_commit_noconflict_parent_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:(@-)+"]); + + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont"]); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 2"]); + std::fs::write(&conflicted_path, "edit not conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["edit", "@-"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()::"]); + + insta::assert_snapshot!(stdout, @r###" + ○ vruxwmqv test.user@example.com 2001-02-03 08:05:15 376a25f2 + │ fixed 2 + @ yqosqzyt test.user@example.com 2001-02-03 08:05:14 f10a05bb + │ fixed 1 + × 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###" + Working copy changes: + M conflicted.txt + Working copy : yqosqzyt f10a05bb fixed 1 + Parent commit: royxmykx a4e88714 (conflict) (empty) boom-cont + Conflict in parent commit has been resolved in working copy + "###); +} + +#[test] +fn test_status_working_commit_conflict_descendant_noconflict() { + 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:(@-)+"]); + + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont"]); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 2"]); + std::fs::write(&conflicted_path, "edit not conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["edit", "@--"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()::"]); + + insta::assert_snapshot!(stdout, @r###" + ○ vruxwmqv test.user@example.com 2001-02-03 08:05:15 376a25f2 + │ fixed 2 + ○ yqosqzyt test.user@example.com 2001-02-03 08:05:14 f10a05bb + │ fixed 1 + @ 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 : royxmykx a4e88714 (conflict) (empty) boom-cont + Parent commit: mzvwutvl 538415e7 (conflict) (empty) boom + Conflict has been resolved in descendants: yqosqzytrlsw + "###); +} + +#[test] +fn test_status_tree_conflict_working_commit_noconflict_descendant_noconflict() { + 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:(@-)+"]); + + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "boom-cont"]); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 2"]); + std::fs::write(&conflicted_path, "edit not conflict").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["edit", "root()+"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()::"]); + + insta::assert_snapshot!(stdout, @r###" + ○ vruxwmqv test.user@example.com 2001-02-03 08:05:15 376a25f2 + │ fixed 2 + ○ yqosqzyt test.user@example.com 2001-02-03 08:05:14 f10a05bb + │ fixed 1 + × 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###" + Working copy changes: + A conflicted.txt + Working copy : qpvuntsm aade7195 Initial contents + Parent commit: zzzzzzzz 00000000 (empty) (no description set) + "###); +} + #[test] fn test_status_simplify_conflict_sides() { let test_env = TestEnvironment::default();