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

On Fri, Nov 21, 2025 at 8:01 AM Junio C Hamano [off-list ref] wrote:

Phillip Wood [off-list ref] writes:

quoted

quoted

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

It was because I didn't think (and I still do not think) your
comments are something for the immediate future, for a tool that
wants to present its minimum-serviceable experimental version to
users so that the users can experiment, extend it more and fix its
behaviour incrementally.  I may have been probably wrong, but I was
getting an impression from the reviews that it is getting to there,
not the feature-perfect version that needs only maintenance from now
on, but a minimum-serviceable one.

We could instead of collect all the I want moon and I want cheeze
comments and iterate until the tool has all of them before it hits
'next', but I do not think it is often what we do to a new feature.

You're reading my feedback as feature requests rather than as bugs
and/or possible paint-ourselves-in-a-corner situations in the
presented implementation?  I must have described things rather poorly;
if they were just feature requests, I'd agree we could just implement
them later.

But maybe I see where the confusion arises, since you were focusing
solely on the single branch thing; that's the one issue where it's
perhaps not as clear whether I was discussing a bug or a new feature.
Let me try to explain that case another way:


The early cover letters said they focused on a case where just a
single branch was involved, yet they don't check whether there really
is only one branch involved for safe operation.

If a user tries to reword or split a commit that is in the history of
multiple branches, the current implementation does not check and makes
the branches diverge.  Some users may want that, though I suspect most
would be negatively surprised.  The commit messages and code do not
even discuss this case.  If we merge the code as-is and then later
notice and fix this problem soon enough, maybe we'd be fine, but I
always worry a bit about a git-switch/git-restore kind of case where
it sits long enough and people depend on side-effects in a way that
prevents us from fixing it.  Besides, since the bug has been
identified and there are multiple not-too-hard ways to fix, I think we
should do something.  Some possibilities:

  * Document this case and warn users to check on their own (not that
friendly, but might be good enough for the first cut).
  * Check if the user-specified commit is part of multiple branches
and error out, unless the user provides a flag verifying that they
want histories to diverge.
  * Just rewrite all relevant branches

The third of those could sound like a feature request in isolation,
but I raised it primarily because it's a potential fix to this
overlooked bug.  I mentioned all three possible fixes, but assumed
others didn't realize how simple that third option was, so I pointed
out how easy it was with some code (~12 lines, which also replace many
more existing lines of code).  I personally think the third option is
*less work* than the second option, and that the focus on trying to
limit to a single branch is creating more work rather than simplifying
the problem.  But if folks really do want to limit to a single branch
despite the code existing to handle the more general case, then we can
implement one of the other solutions.  (If we do so, I still think
choice three is more friendly to users, to cpus, and to future
extension of these features while also simultaneously simplifying the
existing code; so I'll volunteer to investigate and post patches on
top of this series if others decide to go with one of the other
choices for the initial version of the feature.)  My main point here
is just that there is a clear, un-discussed bug, and it should be
addressed somehow in the initial version of the feature.