public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Justin Tobler <jltobler@gmail.com>
To: Toon Claes <toon@iotcl.com>
Cc: git@vger.kernel.org,
	Siddharth Asthana <siddharthasthana31@gmail.com>,
	 Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH RFC] git-replay: implement subcommands
Date: Wed, 11 Mar 2026 13:33:34 -0500	[thread overview]
Message-ID: <abGutGnWo1gN0Dii@denethor> (raw)
In-Reply-To: <20260309-toon-replay-subcommands-v1-1-864ec82ef68a@iotcl.com>

On 26/03/09 08:30PM, Toon Claes wrote:
> git-replay(1) has various operation modes. The mode depends on which of
> the options `--onto`, `--advance`, or `--revert` is given. These options
> are mutually exclusive. This usage pattern is counterintuitive and
> uncommon for Git commands to behave this way.
> 
> Implement subcommands into git-replay(1):
> 
> * `rebase`: This replaces what `--onto=` used to do.

I'm a bit confused by this. It appears that the "rebase" subcommand
still requires the `--onto` option so it doesn't seem to really be
replacing anything. I assume we are tyring to break these operations
into distinct categories which seems reasonable.

> * `pick`: This replaces what `--advance=` used to do.
> * `revert`: This replaces what `--revert=` used to do.
> 
> Option `--onto` is still accepted. It's mandatory for the `rebase`
> subcommand and needs to be used in the exact same way.
> 
> Option `--ref` is added and required for the `pick` and `revert`
> subcommands. This replaces what `--advance` and `--revert` used to do,
> but as a single uniform option for all subcommands.
> 
> The `rebase` subcommand also accepts option `--ref`, and when given this
> is the ref that's updated with the outcome of the git-replay(1) command.
> Thus following commands are identical:
> 
>     $ git replay rebase --onto=master master..branch-1
> 
>     $ git replay rebase --onto=master master..branch-1^{0} --ref=refs/heads/branch-1
> 
> In the second example the upper boundary of the revision range is peeled
> down to a commit (using '^{0}'). Without option `--ref`, git-replay(1)
> doesn't know which ref to update, that's why `--ref` is passed
> explicitly.
> 
> For the subcommands `pick` and `revert` it's also possible to combine
> `--ref` and `--onto`. Here are again two identical examples:
> 
>     $ git replay pick --onto=branch-1 master..aabbccdd
> 
>     $ git replay pick --onto=branch-1^{0} master..aabbccdd --ref=refs/heads/branch-1
> 
> In the latter the argument for `--onto` is peeled down to a commit, so
> the command doesn't know which ref to update. To inform git-replay(1)
> which refs should be updated, it's passed explicitly as option `--ref`.
> 
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> In the patch series by Siddharth Asthana[1] the option `--revert` is
> added to git-replay(1). This is implemented as option `--revert`, next
> to the existing options `--advance` and `--onto`.
> 
> The usage of these options is mutually exclusive, so the user can only
> use one of them, and depending on which one, git-replay(1) selects a
> "mode of operating".
> 
> Various people have raised this behavior is somewhat confusing. In this
> series we attempt to make the usage of git-replay(1) more intuitive and
> user-friendly by implementing the modes as subcommands.

Ok, subcommands for git-replay(1) seem like they could be a good fit
here.

> This patch is submitted as an RFC to gather feedback about the design.
> All changes are implemented as a single patch right now, and thus
> reviewing the changes might be challenging. When we got people aligned
> on the direction, I'll work toward cleaner patches.
> 
> These changes are based on 'master' at 864f55e190 (The second batch,
> 2026-02-09) with the patches of Siddharth[1] applied: 'sa/replay-revert'
> at f79189a653 (replay: add --revert mode to reverse commit changes,
> 2026-02-19)
> 
> [1]: 20260218234215.89326-3-siddharthasthana31@gmail.com
> ---
>  Documentation/git-replay.adoc | 124 ++++++++++++++++----------
>  builtin/replay.c              | 150 ++++++++++++++++++++++++-------
>  replay.c                      |  66 +++++++-------
>  replay.h                      |  31 +++----
>  t/t3650-replay-basics.sh      | 199 +++++++++++++++++++++++-------------------
>  5 files changed, 349 insertions(+), 221 deletions(-)
> 
> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
> index ffdf790278..a7e8dac23f 100644
> --- a/Documentation/git-replay.adoc
> +++ b/Documentation/git-replay.adoc
> @@ -8,8 +8,13 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
>  
>  SYNOPSIS
>  --------
> -[verse]
> -(EXPERIMENTAL!) 'git replay' ([--contained] --onto <newbase> | --advance <branch> | --revert <branch>) [--ref-action[=<mode>]] <revision-range>...

Do we intent to remove the experimental marker?

> +[synopsis]
> +git replay rebase --onto <newbase> [--ref <branch>] [--contained]
> +		[--ref-action[=<mode>]] <revision-range>
> +git replay pick --ref <branch> [--onto <newbase>]
> +		[--ref-action[=<mode>]] <revision-range>
> +git replay revert --ref <branch> [--onto <newbase>]
> +		[--ref-action[=<mode>]] <revision-range>

Subcommands with required options like this feel quite bad IMO and I'm
not sure it makes it much more intuitive. I guess subcommands do make it
easier to convey which options pertain to which operation. Maybe it
would be better if required arguments remained positional though?

Also, using these subcommands appears to be required now which is a
breaking change compared to before. The command is experimental, so this
may be fine, but should probably be more directly mentioned.

-Justin

  reply	other threads:[~2026-03-11 18:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 19:30 [PATCH RFC] git-replay: implement subcommands Toon Claes
2026-03-11 18:33 ` Justin Tobler [this message]
2026-03-13 16:22   ` Toon Claes
2026-03-14  7:18     ` Siddharth Asthana

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abGutGnWo1gN0Dii@denethor \
    --to=jltobler@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=siddharthasthana31@gmail.com \
    --cc=toon@iotcl.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox