Skip to content

revert: default location arguments to -B @#9060

Open
glehmann wants to merge 1 commit intojj-vcs:mainfrom
glehmann:gln/revert-default-location-wkym
Open

revert: default location arguments to -B @#9060
glehmann wants to merge 1 commit intojj-vcs:mainfrom
glehmann:gln/revert-default-location-wkym

Conversation

@glehmann
Copy link
Contributor

@glehmann glehmann commented Mar 8, 2026

Make the location arguments (--onto, --insert-after, --insert-before) optional for revert. This change introduces a default destination of -B @ when no specific location is provided.

Inserting the revert before the current working copy aligns with the common workflow of wanting to immediately build upon or examine the reverted state.

Because this only provides a fallback for previously "missing" arguments, it does not break or change the behavior of existing usages that already provide explicit location flags.

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/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by AI
  • For any prose generated by AI, I have proof-read and copy-edited with an
    eye towards deleting anything that is irrelevant, clarifying anything that
    is confusing, and adding details that are relevant. This includes, for
    example, commit descriptions, PR descriptions, and code comments.

@glehmann glehmann requested a review from a team as a code owner March 8, 2026 20:48
Make the location arguments (--onto, --insert-after, --insert-before)
optional for revert. This change introduces a default destination of -B
@ when no specific location is provided.

Inserting the revert before the current working copy aligns with the
common workflow of wanting to immediately build upon or examine the
reverted state.

Because this only provides a fallback for previously "missing"
arguments, it does not break or change the behavior of existing usages
that already provide explicit location flags.
@glehmann glehmann force-pushed the gln/revert-default-location-wkym branch from e3d07d8 to cb66dab Compare March 8, 2026 20:50
@martinvonz
Copy link
Member

This seems like a good idea to me.

@yuja
Copy link
Contributor

yuja commented Mar 9, 2026

I personally like this default, but I'm not an "edit" workflow user.

@bnjmnt4n
Copy link
Member

bnjmnt4n commented Mar 9, 2026

There’s some prior discussion here: #6358. Like Yuya, I would find this default useful, but I’m not an “edit” workflow user either.

@PhilipMetzger
Copy link
Contributor

I don't like this change to much as an edit workflow user, we have given it no default thus far to not give a workflow a preference.

@martinvonz
Copy link
Member

@PhilipMetzger: What argument do you usually use for the destination? Do you usually do jj revert -r <some old commit> -o @ followed by jj edit @+?

@PhilipMetzger
Copy link
Contributor

What argument do you usually use for the destination?

I usually use --onto 'trunk()' or something similar.

Do you usually do jj revert -r <some old commit> -o @ followed by jj edit @+?

yes, its roughly that.

@martinvonz
Copy link
Member

I'm confused, is it --onto 'trunk()' or --onto @? :) If it's the former, then that doesn't seem very related to edit-vs-new, does it?

@glehmann
Copy link
Contributor Author

glehmann commented Mar 9, 2026

I'm using both workflows depending on what I'm doing, and I'm always using jj revert -r X -B @. In fact there is a single instance in my shell history where I used -d @- by mistake, and I undid it just after.

jj is already quite biased towards the new-squash workflow, and I'm mostly ok with that—just a few places where I'd like jj wouldn't push me out of the edit workflow.
This change would make it easier for the new-squash workflow user, but it wouldn't make the usage harder for the edit workflow users, and wouldn't push anyone out of the edit workflow.

@PhilipMetzger
Copy link
Contributor

I'm confused, is it --onto 'trunk()' or --onto @? :

Its both as I said usually the first one if I'm not sure where I'm at and the second one where its clear what the revert should do.

jj is already quite biased towards the new-squash workflow, and I'm mostly ok with that—just a few places where I'd like jj wouldn't push me out of the edit workflow.

I disagree with that since the only place where you're explicitly asked to create a new commit is when conflicts arise.

This change would make it easier for the new-squash workflow user, but it wouldn't make the usage harder for the edit workflow users, and wouldn't push anyone out of the edit workflow.

While I get your point, its not about "pushing" someone out of the workflow but providing equally unopinionated defaults to users. By making easier for the "new+squash" workflow you make it harder for all "edit" users.

@glehmann
Copy link
Contributor Author

This change would make it easier for the new-squash workflow user, but it wouldn't make the usage harder for the edit workflow users, and wouldn't push anyone out of the edit workflow.

While I get your point, its not about "pushing" someone out of the workflow but providing equally unopinionated defaults to users. By making easier for the "new+squash" workflow you make it harder for all "edit" users.

I don't see why it should be equally "bad" just to be equal for both workflows. And making it easier for the new-squash workflow users wouldn't change anything for the edit workflow users, so it wouldn't make it any harder compared to now. Some use cases would be easier, but none would be harder.

BTW as I said I'm always using jj revert with -B @. What would be a good default for an edit workflow?

@PhilipMetzger
Copy link
Contributor

I don't see why it should be equally "bad" just to be equal for both workflows.

It stays unopinionated which is good, like if we optimized for the "edit" workflow by rebasing it by default on top of @ all "new+squash" users also would be unhappy.

And making it easier for the new-squash workflow users wouldn't change anything for the edit workflow users, so it wouldn't make it any harder compared to now. Some use cases would be easier, but none would be harder.

It does make it harder to stay on top of @ since it will constantly introduce conflicts, that's why its a straight no from me.

What would be a good default for an edit workflow?

^, so either rebasing by default on to @ or trunk().

@glehmann
Copy link
Contributor Author

I don't see why it should be equally "bad" just to be equal for both workflows.

It stays unopinionated which is good, like if we optimized for the "edit" workflow by rebasing it by default on top of @ all "new+squash" users also would be unhappy.

And making it easier for the new-squash workflow users wouldn't change anything for the edit workflow users, so it wouldn't make it any harder compared to now. Some use cases would be easier, but none would be harder.

It does make it harder to stay on top of @ since it will constantly introduce conflicts, that's why its a straight no from me.

What does "stay on top of @" mean?
Why would it constantly introduce conflicts?

What would be a good default for an edit workflow?

^, so either rebasing by default on to @ or trunk().

On top of trunk() seems really strange to me. Because it's the only bookmark we have by default?

Why would you want to revert on top of your current working copy by default?
If you're making changes and notice a regression to revert to, you probably want it before your work.
If you want to switch context and leave your current work aside to revert a revision on something that is already pushed, the top commit of your branch is probably immutable. You have to do a jj new to switch to that branch, and then the revert must be done before @.

@PhilipMetzger
Copy link
Contributor

What does "stay on top of @" mean?
Why would it constantly introduce conflicts?

If my tree looks like this A->B->C->@ in which @ has some changes, doing a revert with this default will potentially introduce conflicts in @ which is undesirable.

On top of trunk() seems really strange to me. Because it's the only bookmark we have by default?

No, because it would reflect of what I'd expect from the command see the discussion above.

Why would you want to revert on top of your current working copy by default?

Because my in-flight changes may be on a different branch entirely or I want to work on the revert separately with edit.

If you want to switch context and leave your current work aside to revert a revision on something that is already pushed, the top commit of your branch is probably immutable. You have to do a jj new to switch to that branch, and then the revert must be done before @.

I mean you can achieve the same thing with a revert followed by a rebase and its likely that I want to edit the reverse diff anyway.

@glehmann
Copy link
Contributor Author

This is why this PR proposes providing a default location. It's not dropping the location flags, and we can still land the revert commit wherever we want.

From where I stand, you are describing difficulties that can occur, but none of them is a good argument against providing a default location.

It seems like there are several ways to look at this. It might be useful if others shared their thoughts to help move things along.

@PhilipMetzger
Copy link
Contributor

This is why this PR proposes providing a default location.

By choosing one workflow over the other which you willfully ignore here.

From where I stand, you are describing difficulties that can occur, but none of them is a good argument against providing a default location.

And you're essentially saying that you don't consider it a good argument and with that disrespecting all users of this workflow. And having this default location will likely cause problems to users of it which you're unable to accept.

Maybe @matts1 also has something to say on this since he's also an user of the edit workflow instead of getting this PR by squash workflow users.

@glehmann
Copy link
Contributor Author

glehmann commented Mar 12, 2026

This is why this PR proposes providing a default location.

By choosing one workflow over the other which you willfully ignore here.

Are you willingly ignoring that I'm also an edit workflow user?

From where I stand, you are describing difficulties that can occur, but none of them is a good argument against providing a default location.

And you're essentially saying that you don't consider it a good argument and with that disrespecting all users of this workflow.

I don't think disagreeing with your arguments is being disrespectful. Are you being disrespectful when you disagree with mine?
And BTW, I'm also an edit workflow user.

And having this default location will likely cause problems to users of it which you're unable to accept.

Again, I disagree with that statement. Could you accept it?

PS: I value the perspective you bring to discussions about jj. However, our exchanges often become unpleasant enough that I find it difficult to continue the conversation.
I may be misreading things. If I’m misunderstanding your intent or communication style, I’m open to clarification and to adjusting how I interpret things.

@martinvonz
Copy link
Member

I think @PhilipMetzger is saying is that the default might make "edit" users accidentally apply the reverted commit before the working copy when they expected to to be on top of the working copy. Is that correct, Philip? I'm a bit surprised that users want the reverted commit on top of the working copy.

@PhilipMetzger
Copy link
Contributor

I think @PhilipMetzger is saying is that the default might make "edit" users accidentally apply the reverted commit before the working copy when they expected to to be on top of the working copy. Is that correct, Philip?

Yes, I also realized that my previous answer can be generalized as wanting the reverted commit to be a sibling of @ either by making it a descendant of trunk() or the working-copy. I also went through this exact workflow for the revert I recently landed.

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.

5 participants