Re: [PATCH v6 00/11] Introduce git-history(1) command for easy history editing

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 21, 2025, 14:31 UTC
Message-ID: <3fb47b15-ed43-4137-95f8-cee97ab5e44c@gmail.com>
In-Reply-To: <CABPp-BEyMFiRdHoseTaYG9rUFO6Ta=dBG88CGRb3CfNf8aSAkg@mail.gmail.com>

On 20/11/2025 22:02, Elijah Newren wrote:

Show 30 quoted lines

> On Thu, Nov 20, 2025 at 12:49 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Elijah Newren <newren@gmail.com> writes:
>>> 
>>> I'm a little unsure if it'll be small or just one reroll.  Some of the
>>> changes for patches 5 & 9 might be big (but straightforward), there's
>>> also a couple design related questions (single branch, HEAD-centric)
>>> that might bring up bigger usability issues to address (if a commit
>>> being edited is part of multiple branches, do we just rewrite all of
>>> them by default, or error out unless the user specifies how they want
>>> it handled)?, and a potential gotcha on patch 11 (how can you preserve
>>> the index and working tree if the user edits the patch while splitting
>>> a commit?) that may require rethinking or restricting that feature.
>>
>> Perhaps.  But I thought the existing patches limited its initial
>> scope small and manageable that by operating only on a single strand
>> of pearls, with an intention to extend to cover more cases later.  I
>> was hoping that we can start small and simple, initially limiting it
>> to single branch, etc., in other areas that require design
>> decisions.
> 
> So, you are referring to the single branch, HEAD-centric piece of the
> feedback.  The funny thing there is that operating on a more limited
> case, without checking and verifying that you are indeed in the more
> limited case (and erroring out if not), risks painting us into a
> corner or providing some really buggy behavior when we aren't actually
> in that case.  To me, it opens a can of worms and makes the problem
> scope bigger instead of smaller.  Funnily enough, the single branch
> thing is also the one piece of this that I think could be solved by a
> fairly small change in the reroll (and I pointed out how in the
> comments), so the limited view really didn't buy anything here IMO.

I can't find that comment. Are you referring to reusing more of the replay machinery? If so we have the problem that the user gives a single commit to "git history" so we don't have a handy revision range to pass to the replay machinery unless we assume we're rewriting an ancestor of HEAD or we go and find all the branches descended from the commit the user gave us. Long term we should certainly do the latter but depending on how much work it is to implement that we may want to go with the single branch case at first

Show 10 quoted lines

> The other problems are independent of whether you try to limit the
> scope initially in such a manner:
> 
> Are the testcases and the code requiring something for the feature
> (ensuring the index and worktree are preserved) doing something that
> is incompatible with the capabilities given to the user (allowing them
> to edit the patch while splitting, so that they stage stuff that
> wasn't part of the original commit)?  Or...is it assumed that the
> split commits always "sum" to the changes in the original commit,
> meaning the "other" patch immediately undoes those extra changes?

Yes that's what's implemented. I think that makes sense for the "split" command. Often when splitting a commit one needs to make small changes to the diff in order for the result to compile but you still want the same end state from the sum of the split commits.

Show 6 quoted lines

> I'm also worried about extended header handling for the edited
> (reworded or split) commits.  That seems to have been overlooked in
> this series, despite the fact that in early versions extended headers
> were explicitly called out for the remainder of the commits being
> replayed/rebased, so it seems interesting that they weren't considered
> for the commits explicitly being edited.

What headers does it make sense to copy when splitting a commit? When rewording it is more likely that copying the extended headers is what the user wants but the example of the "encoding" header you gave does not make sense to me as we re-encode the commit message and author data when the user edit's the message so we're not preserving the original encoding.

> And I'm a bit surprised that the original commit message for a split
> commit is automatically associated with the second commit; if I had
> been forced to choose, I would have assumed it should be associated
> with the first.

I don't think it is safe to assume either - we should prompt the user to edit the message when creating both commits and seed the editor with the original message.

Show 6 quoted lines

> Granted, I think good progress is being made and perhaps the changes
> needed for the rest aren't that huge (and maybe there's more pieces
> I'm not quite understanding yet similar to the
> two-split-patches-always-summing-to-the-original), I was just a little
> surprised that my comments are summarized by "expecting a small and
> final reroll".  :-)

Yes I'm not expecting any new functionality but I am expecting a bit more than tiny cleanup.

Thanks

Phillip