Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polish/4147: Change conflict hint if decendants resolves conflict. #4185

Merged
merged 1 commit into from
Aug 1, 2024
Merged

polish/4147: Change conflict hint if decendants resolves conflict. #4185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
essiene marked this conversation as resolved.
Show resolved Hide resolved
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