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.

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

Conversation

essiene
Copy link
Contributor

@essiene essiene commented Jul 31, 2024

Fixes #4147

Change behaviour to consider the state of the working commit, instead of the state of the tree. In particular:

  • If working commit has conflicts: Print the existing message we print today.
  • If working commit has no conflicts:
    • If any parent has conflicts, print: "Conflict in parent is resolved in working copy"
    • If no parent has any conflicts: no message

This behaviour resolves the issue raised in #4147.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have added tests to cover my changes

@essiene essiene force-pushed the polish/4147 branch 5 times, most recently from 6428b9e to dce3dab Compare July 31, 2024 02:05
@essiene essiene marked this pull request as ready for review July 31, 2024 02:11
@essiene essiene requested a review from martinvonz July 31, 2024 02:13
lib/src/default_index/revset_engine.rs Outdated Show resolved Hide resolved
cli/src/commands/status.rs Outdated Show resolved Hide resolved
@essiene essiene force-pushed the polish/4147 branch 2 times, most recently from 12c5172 to 476b24e Compare July 31, 2024 20:33
@essiene essiene requested a review from yuja July 31, 2024 20:40
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. We might want to handle wc_parent == root_conflict better, but this PR looks strictly better than the current state.

cli/src/commands/status.rs Outdated Show resolved Hide resolved
cli/src/commands/status.rs Outdated Show resolved Hide resolved
cli/tests/test_status_command.rs Show resolved Hide resolved
cli/tests/test_status_command.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@essiene essiene force-pushed the polish/4147 branch 3 times, most recently from 7201d40 to a12a187 Compare August 1, 2024 10:35
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
@essiene essiene enabled auto-merge (rebase) August 1, 2024 15:12
@essiene essiene merged commit 7c4185c into main Aug 1, 2024
29 checks passed
@essiene essiene deleted the polish/4147 branch August 1, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"jj status" output while fixing conflicts is misleading and offers bad advice
2 participants