Re: [PATCH v5 0/4] history: add squash subcommand to fold a range

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

On 29/06/2026 19:03, Harald Nordgren wrote:
>> So instead of
>>
>>       # This is the combination of 4 commits
>>       # This is the first commit message
>>       Base subject
>>
>>       Base body
>>
>>       # This is the second commit message
>>       # Another subject
>>
>>       # Another body
>>
>>       # This is the third commit message
>>       # fixup! Base subject
>>
>>       # This is the fourth commit message
>>       # amend! Another subject
>>       A better subject
>>
>>       A better body
>>
>> We'd have
>>
>>       # This is the combination of 4 commits
>>       # 123 Base subject
>>       # 456 Another subject
>>       # 789 fixup! Base subject
>>       # abc amend! Another subject
>>
>>       Base Subject
>>
>>       Base Body
>>
>>       Another subject
>>
>>       Another Body
> 
> I think this makes it a lot harder to read. If every commit body was
> always just a single paragraph it could make sense, but it's generally
> not. Look at the commits in this series, with no delimiter of where
> one commit message ends and the next one starts, it would be very
> confusing.

You've trimmed the line where I said

 >> Possibly with a comment before each message saying where it came
 >> from.

So I'm not against adding a comment before each message, but I do think 
we should omit any messages that would be commented out completely. If 
you look at the rebase example above you can see there is a mass of 
comments between the two pieces of text that make up the new message. 
That makes it hard to see what is actually going to be included in the 
new message.

Thanks

Phillip

>> It would be good to error out if the user tries squash a fixup! style
>> commit and range does not contain its target commit.
> 
> Good point, I don't see a good reason to allow this.
> 
>> There does seem to be some support for merges in this patch series which
>> I think behaves pretty sensibly. If we have
>>
>>             C - D
>>            /     \
>>     - A - B - E - F - G
>>
>> Then squashing A..G should be fine because the parents of F are in the
>> range and it looks like we support that. Squashing should B..G without
>> --ancestry-path should be safe as well because B ends up as the parent
>> of the squashed commit but we don't have a way to disable
>> --ancestry-path (and maybe we don't want to add one). Squashing F^@..G
>> might be useful to fixup a merge (though perhaps amending F rather than
>> creating G is a simpler way to fix a broken merge). Squashing E..G does
>> not make sense because the range does not include one of the merge parents.
> 
> Thanks for a good explanation here!
> 
> 
> Harald
>