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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 22, 2019, 15:49 UTC
Message-ID: <xmqqftltqjy1.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1908221418540.46@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

>> As with this commit the reset must never change the active branch,
>> the 'HEAD is now at ...' message has now been removed.
>
> Actually, I am not so sure that I like this change.
>
> Previously, users had a chance to figure out to which revision the
> worktree was reset, before switching the branch (because switching the
> branch we _do_, via `git checkout master`).

Hmph, that only happens when --autostash is in effect and actually had created a stash, no? If your working tree is clean, or if you did not pass --autostash, "HEAD is now at ..." is not reported. I am not sure why that particular piece of information is only useful in the case we actually created a stash and unnecessary if we did not create a stash.

When we do not create a stash, the output starts from "First, rewinding head to replay your work on top of it...", which sort of gives a warm and fuzzy impression that it is reporting what it is doing, but without giving the most useful information (i.e. what "it" refers to).

Because I am all for preserving the existing behaviour as much as possible when fixing real bugs, I would not strongly object to your idea of resurrecting the message. But I am not sure if the existing message was all that useful in the first place. I'd rather see these messages that were only emitted when --autostash was given removed first (like this patch does), and then the "First rewinding..." message reworded to show where we rebuilt the history on top of.

Other than that, thanks for a good review, and thanks Ben for working on this.