Re: [PATCH 1/1] replay: add --revert option to reverse commit changes

From: Elijah Newren <newren@gmail.com>
Date: Nov 26, 2025, 23:57 UTC
Message-ID: <CABPp-BESM4PC+QVXZ-X_Y0m3PrSQGuc-jfB2pCJ+hXy0Gi-T5A@mail.gmail.com>
In-Reply-To: <xmqqy0nsl741.fsf@gitster.g>

On Wed, Nov 26, 2025 at 3:14 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

>
> Elijah Newren <newren@gmail.com> writes:
>
> > == Example command lines from your proposal ==
> >
> >    git replay --rebase main feature~3..feature
> >
> > This command to me would suggest that main is being rebased, but it
> > isn't -- it rebases feature~3..feature onto main while updating
> > feature to point at the result.  I find the "--rebase main" part of
> > this command line confusing.
> >
> >    git replay --cherry-pick main feature~3..feature
> >
> > This command to me would suggest that main is being cherry-picked, but
> > it isn't -- it cherry-picks feature~3..feature onto main while
> > updating main to point at the result.  Again, I find the
> > "--cherry-pick main" part of this command line confusing.
>
> That only tells us that if you want to help users by limiting the
> vocabulary to a single set (i.e. both command names, and mode names
> used in replay), you'd need to make sure you have the order of
> <branch> and <range> given to the replay command in logical order,
> in line with the option name, no?  Of course, if you want to say
> "cherry-pick", cherry-picked range would have to come near the
> option flag that says "cherry-pick", naturally.

--advance and --onto are flags that require an argument -- in this case, "main". So, now you're suggesting more than renaming, in particular some bigger refactoring such as making these flags now require the <range> rather than the <base>. Let's follow that path a bit further...

Does your proposal assume that <range> is simple, such as "feature~3..feature" above (i.e. something that an argument parser would view as a single argument)? What if the <range> were "^main feature1 feature2"? Or ""^$COMMIT --ancestry-path --branches"? (I don't see how to have the option parser easily be able to stuff the arguments to "--rebase ^$COMMIT --ancestry-path --branches" into a range variable that eats all of "^$COMMIT --ancestry-path --branches".) While I use simple ranges to describe the feature, I specifically built the command to be able to do things like those other two examples and use it for those. Those more complicated examples are things the rebase command just can't do.

Also, just like `git log` allows `git log [<options>] [<revision range>]`, I wanted git replay to allow `git replay [<options>] [<revision range>]`. Instead of doing magic to get an implicit revision range as rebase does (and with rather limited options because of that magic), suddenly people can use what they've learned from `git log` in another place. But that piece of knowledge only really transfers if we do similarly to `git log`, i.e. the revision range comes after other options.

Perhaps one way to avoid the first problem above is to make `--onto/--advance/--rebase/--cherry-pick" stop requiring (or accepting) an argument and turn them into simple mode toggles, and then make both <base> and <range> be positional arguments, with some well-defined ordering. However, if <base> comes before <revision> then we still have the same problem as my previous email, whereas if it comes after, then we weaken or destroy the connection to `git log` I made above. Maybe the connection to `git log` isn't that important. What I think is important either way, though, is if we use positional arguments for both things instead of making (at least one) an option, then I feel we are copying one of the designs of `git rebase` that makes it hard for even me to use: I hate that it uses multiple positional arguments to define the operation; despite using the command heavily for 16-17 years and sending in lots of patches to improve it, I still can't remember the order of those positional arguments and have to look it up again when teaching others. Maybe that's a personal shortcoming, but I would really rather that either <base> or <revision> was an option flag.