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 6428b9e
Show file tree
Hide file tree
Showing 3 changed files with 342 additions and 18 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
Loading

0 comments on commit 6428b9e

Please sign in to comment.