From 5361c7388caca2acbc71473ea5d15f66b1f292a3 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: * Continue printing the same message we print today. * If working commit has no conflicts: * If any parent has conflicts, we print: "Conflict in parent is resolved in working copy". Also 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. * Update unittests for conflict hinting update. * Update CHANGELOG --- CHANGELOG.md | 5 ++ cli/src/commands/status.rs | 45 ++++++++---- cli/tests/test_status_command.rs | 115 ++++++++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1444c566b..14640d57d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,11 @@ 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 fixes + a conflict in the parent, 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 490aba5436..7869f9f57a 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -92,21 +92,36 @@ 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()); + + // 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)?; + } else { + for parent in wc_commit.parents() { + let parent = parent?; + if parent.has_conflict()? { + writeln!( + formatter.labeled("hint"), + "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 81bb9e0372..1e84bb9316 100644 --- a/cli/tests/test_status_command.rs +++ b/cli/tests/test_status_command.rs @@ -105,8 +105,9 @@ fn test_status_filtered() { } // See +// See #[test] -fn test_status_display_rebase_instructions() { +fn test_status_display_relevant_working_commit_conflict_hints() { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -143,7 +144,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", "::"]); insta::assert_snapshot!(stdout, @r###" @ yqosqzyt test.user@example.com 2001-02-03 08:05:13 65143fef conflict @@ -175,6 +176,116 @@ fn test_status_display_rebase_instructions() { Once the conflicts are resolved, you may want to inspect the result with `jj diff`. Then run `jj squash` to move the resolution into the conflicted commit. "###); + + // Resolve conflict + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 1"]); + std::fs::write(&conflicted_path, "first commit to fix conflict").unwrap(); + + // Add one more commit atop the commit that resolves the conflict. + test_env.jj_cmd_ok(&repo_path, &["new", "--message", "fixed 2"]); + std::fs::write(&conflicted_path, "edit not conflict").unwrap(); + + // wc is now conflict free, parent is also conflict free + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "::"]); + + insta::assert_snapshot!(stdout, @r###" + @ kpqxywon test.user@example.com 2001-02-03 08:05:18 3432159f + │ fixed 2 + ○ znkkpsqq test.user@example.com 2001-02-03 08:05:17 897d589f + │ fixed 1 + × yqosqzyt test.user@example.com 2001-02-03 08:05:13 65143fef conflict + │ (empty) boom-cont-2 + × 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 : kpqxywon 3432159f fixed 2 + Parent commit: znkkpsqq 897d589f fixed 1 + "###); + + // Step back one. + // wc is still conflict free, parent has a conflict. + test_env.jj_cmd_ok(&repo_path, &["edit", "@-"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "::"]); + + insta::assert_snapshot!(stdout, @r###" + ○ kpqxywon test.user@example.com 2001-02-03 08:05:18 3432159f + │ fixed 2 + @ znkkpsqq test.user@example.com 2001-02-03 08:05:17 897d589f + │ fixed 1 + × yqosqzyt test.user@example.com 2001-02-03 08:05:13 65143fef conflict + │ (empty) boom-cont-2 + × 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 : znkkpsqq 897d589f fixed 1 + Parent commit: yqosqzyt 65143fef (conflict) (empty) boom-cont-2 + Conflict in parent commit has been resolved in working copy + "###); + + // Step back to all the way to `root()+` so that wc has no conflict, even though + // there is a conflict later in the tree. So that we can confirm + // our hinting logic doesn't get confused. + test_env.jj_cmd_ok(&repo_path, &["edit", "root()+"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "::"]); + + insta::assert_snapshot!(stdout, @r###" + ○ kpqxywon test.user@example.com 2001-02-03 08:05:18 3432159f + │ fixed 2 + ○ znkkpsqq test.user@example.com 2001-02-03 08:05:17 897d589f + │ fixed 1 + × yqosqzyt test.user@example.com 2001-02-03 08:05:13 65143fef conflict + │ (empty) boom-cont-2 + × 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]