Thread (87 messages) 87 messages, 7 authors, 1d ago

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

From: Phillip Wood <hidden>
Date: 2026-07-07 09:48:20

Hi Harald

On 07/07/2026 08:51, Harald Nordgren wrote:
quoted
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.
quoted
quoted
     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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help