Skip to content

Commit

Permalink
Change conflict hint depending on state of working commit.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
essiene committed Aug 1, 2024
1 parent d0f6f42 commit 7c4185c
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 17 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 30 additions & 15 deletions cli/src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
}
Expand Down
115 changes: 113 additions & 2 deletions cli/tests/test_status_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ fn test_status_filtered() {
}

// See <https://github.com/martinvonz/jj/issues/3108>
// See <https://github.com/martinvonz/jj/issues/4147>
#[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"]);

Expand Down Expand Up @@ -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 [email protected] 2001-02-03 08:05:13 65143fef conflict
Expand Down Expand Up @@ -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 [email protected] 2001-02-03 08:05:18 3432159f
│ fixed 2
○ znkkpsqq [email protected] 2001-02-03 08:05:17 897d589f
│ fixed 1
× yqosqzyt [email protected] 2001-02-03 08:05:13 65143fef conflict
│ (empty) boom-cont-2
× royxmykx [email protected] 2001-02-03 08:05:12 a4e88714 conflict
│ (empty) boom-cont
× mzvwutvl [email protected] 2001-02-03 08:05:11 538415e7 conflict
├─╮ (empty) boom
│ ○ kkmpptxz [email protected] 2001-02-03 08:05:10 1e8c2956
│ │ First part of conflicting change
○ │ zsuskuln [email protected] 2001-02-03 08:05:11 2c8b19fd
├─╯ Second part of conflicting change
○ qpvuntsm [email protected] 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 [email protected] 2001-02-03 08:05:18 3432159f
│ fixed 2
@ znkkpsqq [email protected] 2001-02-03 08:05:17 897d589f
│ fixed 1
× yqosqzyt [email protected] 2001-02-03 08:05:13 65143fef conflict
│ (empty) boom-cont-2
× royxmykx [email protected] 2001-02-03 08:05:12 a4e88714 conflict
│ (empty) boom-cont
× mzvwutvl [email protected] 2001-02-03 08:05:11 538415e7 conflict
├─╮ (empty) boom
│ ○ kkmpptxz [email protected] 2001-02-03 08:05:10 1e8c2956
│ │ First part of conflicting change
○ │ zsuskuln [email protected] 2001-02-03 08:05:11 2c8b19fd
├─╯ Second part of conflicting change
○ qpvuntsm [email protected] 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 [email protected] 2001-02-03 08:05:18 3432159f
│ fixed 2
○ znkkpsqq [email protected] 2001-02-03 08:05:17 897d589f
│ fixed 1
× yqosqzyt [email protected] 2001-02-03 08:05:13 65143fef conflict
│ (empty) boom-cont-2
× royxmykx [email protected] 2001-02-03 08:05:12 a4e88714 conflict
│ (empty) boom-cont
× mzvwutvl [email protected] 2001-02-03 08:05:11 538415e7 conflict
├─╮ (empty) boom
│ ○ kkmpptxz [email protected] 2001-02-03 08:05:10 1e8c2956
│ │ First part of conflicting change
○ │ zsuskuln [email protected] 2001-02-03 08:05:11 2c8b19fd
├─╯ Second part of conflicting change
@ qpvuntsm [email protected] 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]
Expand Down

0 comments on commit 7c4185c

Please sign in to comment.