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

From: Elijah Newren <newren@gmail.com>
Date: Nov 20, 2025, 22:02 UTC
Message-ID: <CABPp-BEyMFiRdHoseTaYG9rUFO6Ta=dBG88CGRb3CfNf8aSAkg@mail.gmail.com>
In-Reply-To: <xmqq7bvk77lr.fsf@gitster.g>

On Thu, Nov 20, 2025 at 12:49 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 37 quoted lines

>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Thu, Nov 20, 2025 at 12:28 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Elijah Newren <newren@gmail.com> writes:
> >>
> >> >> This patch series is a starting point for such a command. I've
> >> >> significantly slimmed it down from the first couple revisions now
> >> >> following the discussions at the Contributor's Summit yesterday. This
> >> >> was my intent anyway, as I already mentioned on the last iteration.
> >> >
> >> > Sorry for taking so long to review the series now that it's based on
> >> > replay.  Thanks for working on this!
> >>
> >> With your comments and Phillip's, it seems that we are very close to
> >> a good stopping point.  Let me mark the topic as expecting a
> >> hopefully small and final reroll before getting ready for 'next'.
> >>
> >> Thanks, all.
> >
> > 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.

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? (Perhaps it's the latter, which I didn't think of until now, so maybe we are closer to a solution than I realized. In fact, re-reading the code that looks like it does do that and I just missed it. But, perhaps having users edit the patch when splitting commits is a special case that should be called out in the docs, since that might surprise users who try it?)

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.

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.

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". :-)