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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Harald

On 07/07/2026 08:51, Harald Nordgren wrote:
>> 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.

>> 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