Re: [PATCH] Add abbreviated commit hash to rebase conflict message

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2011, 00:12 UTC
Message-ID: <7v4nyg6b9s.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAGdFq_gS2fV5B26ZBOLs=5L_rnaeORmKW49OxwbP-+vx+ZN8cQ@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

Show 13 quoted lines

> On Sun, Nov 6, 2011 at 21:27, Junio C Hamano <gitster@pobox.com> wrote:
>> In what situation does it make sense to say "It came from _this_ commit"?
>>
>> I think there is a separate variable that allows any part of the script if
>> we are being run as a backend of rebase or not, and that is the condition
>> you are looking for.
>
> The closest I could find is:
>
>                 if test -f "$dotest/rebasing"
>
> Which is exactly the case when commit is set. Do you prefer the "-f
> $dotest/rebasing" test or the "-n $commit" one?

Given the variable scoping rules of vanilla shell script, relying on the variable $commit is a very bad idea to begin with. I think the variable also is used to hold the final commit object name produced by patch application elsewhere in the script in the same loop, and I do not think existing code clears it before each iteration, as each part of the exiting code uses the variable only immediately after that part assigns to the variable for its own purpose, and they all know that nobody uses the variable as a way for long haul communication media between different parts of the script. Unless your patch updated that aspect of the lifetime rule for the variable, which I doubt you did, using $commit would introduce yet another bug without solving anything, I would think.