Re: [PATCH v4 1/1] rebase.c: make sure the active branch isn't moved when autostashing

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

Junio C Hamano <gitster@pobox.com> writes:

> IIUC, the bug is twofold:
> ...
>  - A message is given only when the above happens.  When rebasing
> ...
>    That "reset --hard" is done only to clean the index and the
>    working tree and talking about "HEAD is now..." is a bug in its
>    context.

Actually, this "latter" bug can further be split into two

 * The "HEAD is now" is given only when autostash feature needs to
   clean the working tree, and we have never moved HEAD anyway.

 * The message does not indicate what we are rebuilding on top of.

and dealt with separately, so with that in mind the step that would
follow the first patch, i.e.

> ... update
> this patch to add logic to give a pointless and misleading "HEAD is
> now at..." message so that we will report the location of HEAD where
> the "rebase --autostash" command started at, to fix only the first
> bug.

may become different.  The fact that the "HEAD is now..." is given
only when autostash actually happens _might_ be taken as a feature
by some users---the location of HEAD reported by the message is
irrelevant to them (we know that as a fact---we have been reporting
a wrong commit all along anyway), but the single-bit "we got a
message" is a signal that "--autostash" had something valuable to
save.

So the second step may be to replace the "HEAD is now..." message we
add back (relative to Ben's patch under discussion) to the first
patch with a more direct "stashed away your local changes" message
(perhaps with diffstat???  I do not care about the details, as we
are talking about resurrecting one single useful bit of information
and extending it futher is beyond the scope of this analysis).

And the last point, i.e. "First, rewinding head to replay your
work..." does not give enough information to be truly useful, is a
totally separate bug (that Ben's patch does not even mention or
attempt to address), so we can leave it out of this analysis, too.

So, yeah, if we are to spend extra effort to polish Ben's patch
further while keeping the "fix things without making unnecessary
changes", I think the approach that takes least amount of effort may
not to make the code manually say "Head is at ...", but to add a new
message to report that autostash happened.  That fixes two bugs
(i.e. the original bug, and the "we autostashed" bit is reported in
a roundabout and misleading way via "HEAD is now at ...") in a
single patch ;-)