Re: [PATCH v7 0/5] history: add squash subcommand to fold a range

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jul 7, 2026, 09:30 UTC
Message-ID: <38493ca6-8fdd-4b6c-9972-5145f3bf0aa4@gmail.com>
In-Reply-To: <CAHwyqnVd2OsmD-Y4YKVr9GsYdHRRNDot5EKSSESoRM-mf82YSg@mail.gmail.com>

Hi Harald

On 07/07/2026 09:55, Harald Nordgren wrote:

Show 11 quoted lines

>>> The range-diff does not show any input sanitization - what happens when
>>> the user passes "--reverse" for example? As I said in [4] we should copy
>>> what "git replay" does to sanity check the rev-list options, otherwise
>>> we've got no idea whether the parent of the first commit returned by
>>> get_revision() is the commit we want to use as the parent of the
>>> squashed commit.
>>
>> Yeah, good point.
> 
> Well, the code already blocks "--reverse" and other unknown options,
> but I can clarify that better in the commit message.

Well it accepts

	git history squash -- --reverse ...

because after calling parse_options() everything after the "--" is passed to setup_revisions(). There was some discussion about accepting rev-list options [2] so it would have been helpful to reference that in the cover letter. The cover letter should explain both the changes you have made and the suggestions that were discussed that have not been implemented so readers can get an overview of how this version relates to the previous discussion. Without that it is impossible to know if you disagree with a suggestion or have just forgotten it.

"git replay" supports arbitrary rev-list options by passing

      PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT

to parse_options(), then passing the remaining options to setup_revisions(). After that it checks the various members of `struct rev_info` that it cares about are still set appropriately.

Thanks

Phillip

[1] https://lore.kernel.org/git/xmqqzf0dwalx.fsf@gitster.g