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:48 UTC
Message-ID: <40add226-d986-409d-9d9e-d14a56e62f27@gmail.com>
In-Reply-To: <CAHwyqnVxa34iGmFvL4Ujrc2dTbmtF+7j7b=q5v95y=-pzUf0NA@mail.gmail.com>

Hi Harald

On 07/07/2026 08:51, Harald Nordgren wrote:

Show 22 quoted lines

>> There was some discussion [1] about making that the default and renaming
>> it - was that overlooked? If not it would be helpful to comment on those
>> discussions to explain why you don't think it is a good idea.
> 
> Not overlooked, but I side-stepped it because the discussion died
> down, and yes I don't agree that it needs to be the default. I could
> have mentioned my thinking in the cover letter.
> 
>>>      now builds the same editor template git rebase -i shows
>>>      for a squash (a combination of N commits banner with each folded message
>>>      under its own header) and follows autosquash for markers: a fixup!
>>>      message falls out (commented under a will be skipped header), while a
>>>      squash! or amend! keeps its body with only the marker subject commented
>>>      so its remark can be reworded in. Only the message text is affected,
>>>      every commit's changes are always folded in.
>>
>> Rebase re-orders commits so that fixups immediately follow their target
>> - do you do that here? I think that is very relevant because here we may
>> be dealing with several different commits each being targeted by a set
>> of fixups and presenting them mixed together will be confusing.
> 
> No, I'm not doing that now, but I can take a look at that.

That's great, it is fine to punt things like this which require quite a bit of work to implement to a later re-roll but please be clear in the cover letter so reviewers know what to expect.

Show 10 quoted lines

>> I think it should allow squashing a bunch of fixups together though. I
>> thought there was a plan [3] to refuse to squash a fixup unless the
>> range included its target.
> 
> I attempted this with reject_fixupish_oldest(), assuming only the
> first commit needs to be checked as not being a fixup/squash/amend.
> 
> But now I realize that maybe we need to check all of the commits, and
> also check if the target is in the range or not. It just makes the
> logic a lot bigger.

Yes it is a bit more involved. If the first commit is a fixup! then we should allow the user to squash other fixups with the same target and take the message from the last "amend!" commit if we see one. If there are other commits it the range then we should refuse to squash as you do here.

If the first target is not a fixup then we should refuse fixup commits whose target we have not seen. As well as exact subject matches "git rebase" accepts prefix matches and "fixup! $objectid". I think it is fine to skip the prefix matches to start with here. The $objectid matches shouldn't be too much extra work and I think they are worth supporting because if I remember correctly git-gui creates them. Another gotcha is that fixuping up a fixup prepends a "fixup!" to the subject line so you need to be able to handle things like

	fixup! fixup! the real target
	fixup! amend! the real target
	squash! fixup! the real target

etc. Hopefully looking at the code that handles fixups in the sequencer will help

Thanks

Phillip