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:
  * if any decendants are conflict free: "Conflict fixed in <decendant>". 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
  • Loading branch information
essiene committed Jul 31, 2024
1 parent 129aca4 commit 9ceb70d
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 17 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 49 additions & 16 deletions cli/src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<Vec<_>>()
.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")?;
}
Expand Down
77 changes: 76 additions & 1 deletion cli/tests/test_status_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn test_status_filtered() {

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

Expand Down Expand Up @@ -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 <PARENT>
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 <PARENT>
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 <CHILD1> and <CHILD2>
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 [email protected] 2001-02-03 08:05:14 c6ea5f6c
│ (empty) child of conflict fixing commit
○ yqosqzyt [email protected] 2001-02-03 08:05:14 15837663
│ fix_conflict1
× 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###"
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();
Expand Down

0 comments on commit 9ceb70d

Please sign in to comment.