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

Jump to bottom

Make next/prev infer --edit from running on non-head #3028

Merged
merged 2 commits into from
Feb 12, 2024
Merged

Make next/prev infer --edit from running on non-head #3028

merged 2 commits into from
Feb 12, 2024

Conversation

martinvonz
Copy link
Member

@martinvonz martinvonz commented Feb 12, 2024 •
edited
Loading

Checklist

If applicable:

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

PhilipMetzger
PhilipMetzger approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

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

LGTM, still pondering if we should make it configurable.

cli/src/commands/prev.rs Outdated Show resolved Hide resolved
cli/src/commands/next.rs Outdated Show resolved Hide resolved
@martinvonz
next/prev: make --edit implied when already on non-head commit
1236c39 
Users who edit non-head commits usually expect `jj next/prev` to
continue to edit the next/previous commit, so let's make that the
default behavior. This should not confuse users who don't edit
non-head commits since they will simply not be in this state. My main
concern is that doing `jj next; jj prev` will now usually take you
back to the previous commit, but not if you started on the parent of a
head commit.
@martinvonz martinvonz enabled auto-merge (rebase) February 12, 2024 18:40
@martinvonz martinvonz merged commit af8eb3f into main Feb 12, 2024
15 checks passed
@martinvonz martinvonz deleted the push-xyyukrsrkkoz branch February 12, 2024 18:42
@emesterhazy
Copy link
Contributor

emesterhazy commented Apr 8, 2024 •
edited
Loading

The problem I have with this is that if I jj new an interior commit and then jj squash it, the working copy will be changed to the commit that I squashed my changes into. As a result, jj next and jj prev don't work as I might hope.

But, you can always run jj new after so it's not a huge issue. Plus, I don't remember using next or prev before this PR, but since @:: was excluded from the targets for jj next it seems like that could lead to a strange UX where @ is in the middle of a linear history and jj next refuses to move.

So maybe the new behavior is fine.

@emesterhazy emesterhazy mentioned this pull request Apr 8, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilipMetzger PhilipMetzger PhilipMetzger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinvonz @emesterhazy @PhilipMetzger