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

Make jj prev and jj next work when the working copy is a merge commit #3461

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

emesterhazy
Copy link
Collaborator

@emesterhazy emesterhazy commented Apr 5, 2024

Before this commit jj prev fails if the current working copy commit is a merge commit. After this commit it will prompt the user to choose the ancestor they want to select.

#2126

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have added tests to cover my changes
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)

@emesterhazy
Copy link
Collaborator Author

@PhilipMetzger I think that this just works, and that the logic is actually a bit simpler now. You are probably the expert for this command though. Do you think there are any other scenarios we should test?

@emesterhazy emesterhazy changed the title Make jj prev work when @ is a merge commit Make jj prev work when the working copy is a merge commit Apr 5, 2024
@emesterhazy emesterhazy force-pushed the push-yssoqtwsntox branch 2 times, most recently from c3ebf6c to 9909050 Compare April 5, 2024 18:41
@martinvonz
Copy link
Member

It looks like jj next could use the same fix. Feel like taking care of that too?

@emesterhazy
Copy link
Collaborator Author

Yeah I can take a look at next next. Should it go into this PR as a separate commit, or should we merge this first? I'm fine either way.

@martinvonz
Copy link
Member

Yeah I can take a look at next next. Should it go into this PR as a separate commit, or should we merge this first? I'm fine either way.

I would do it in the same PR but up to you since you're the one doing it.

cli/src/commands/prev.rs Outdated Show resolved Hide resolved
@emesterhazy emesterhazy changed the title Make jj prev work when the working copy is a merge commit Make jj prev and jj next work when the working copy is a merge commit Apr 8, 2024
@emesterhazy
Copy link
Collaborator Author

I pushed a second commit that removes the merge commit limitation from jj next as well.

Copy link
Collaborator

@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.

lgtm, thanks.

cli/src/commands/prev.rs Outdated Show resolved Hide resolved
cli/src/commands/prev.rs Show resolved Hide resolved
cli/tests/test_next_prev_commands.rs Outdated Show resolved Hide resolved
cli/src/commands/next.rs Show resolved Hide resolved
cli/src/commands/prev.rs Show resolved Hide resolved
emesterhazy added a commit that referenced this pull request Apr 8, 2024
…ts_at`

The current documentation is wrong. This is a follow up for
#3461 (comment)
emesterhazy added a commit that referenced this pull request Apr 8, 2024
…ts_at`

The current documentation is wrong. This is a follow up for
#3461 (comment)
Copy link
Collaborator

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

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

LG

emesterhazy added a commit that referenced this pull request Apr 8, 2024
…ts_at`

The current documentation is wrong. This is a follow up for
#3461 (comment)
Base automatically changed from push-qwxwsrzwqtxy to main April 8, 2024 13:12
Before this commit `jj prev` fails if the current working copy commit is a
merge commit. After this commit it will prompt the user to choose the ancestor
they want to select.

#2126
@emesterhazy
Copy link
Collaborator Author

Thanks everyone for helping to review this!

@emesterhazy emesterhazy merged commit 13592ce into main Apr 8, 2024
16 checks passed
@emesterhazy emesterhazy deleted the push-yssoqtwsntox branch April 8, 2024 18:52
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.

4 participants