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

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> It would be good to error out if the user tries squash a fixup! style 
> commit and range does not contain its target commit.

That's interesting.  It at least deserves a warning, even though the
choice to leave them out, with intentions to make it a separate fix
on top, may be deliberate.  If such a change-of-mind-in-the-middle
is rare, then erroring out would be perfect.  The user can first
reword the !fixup to make it a separate fix and then squash the
range in quesiton.

> We should sanitize what the user passes though - we do not want to 
> accept arbitrary rev-list options. Off the top of my head "--left-only" 
> and "--right-only" would allow the use of "A...B" and allowing "--not" 
> seems reasonable.

I would not recommend guessing what these rev-list "expressions"
would produce and blacklist some of the operations and notations.
It would be a more robust approach to let the machinery do its thing
to determine the set of commits, *and* inspect the shape of the
history these commits represent.  Are they connected?  Do they have
a single "bottom" that is just outside and below the range so that
we can replace it with the result of squashing everything together?
Do they have a single "top" whose children can be rewritten to have
the resulting single commit as one of their parents?  Starting from
the acceptable shape of the history we want to deal with, rather
than trying to enumerate rev-list operations and notations that
would prevent the resulting set of commits to fall outside the
acceptable shape of the history (and I am reasonably sure anybody
who attempts to do so would either end up with unusablly narrow
subset of what we can reasonably handle, or miss some cases that we
do not want to handle), would be a better approach.