Re: [PATCH] difftool: honor --trust-exit-code for builtin tools

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

Mikael Magnusson <mikachu@gmail.com> writes:

>>> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
>>> index a40d3df..2b66351 100644
>>> --- a/git-mergetool--lib.sh
>>> +++ b/git-mergetool--lib.sh
>>> @@ -221,6 +221,7 @@ run_merge_tool () {
>>>       else
>>>               run_diff_cmd "$1"
>>>       fi
>>> +     status=$?
>>>       return $status
>>>  }
>>
>> Thanks for a quick turn-around.  As a hot-fix for what is already in
>> -rc I am fine with this fix but the patch makes me wonder if $status
>> as a global shell variable has any significance.
>
> $status is an alias for $? in zsh, and so cannot be assigned to. But
> other than that I don't think it holds any meaning and should be fine
> in a .sh script.

That is not what I meant by "global ... significance".

The question was if the codepath in the caller depends on this
setting the global variable here, or nobody looks at and depends on
the global variable we are setting here after this function returns.

It does not have any significance that a random shell implementation
is not POSIX compliant.  That would merely mean that such a shell
cannot be used to run POSIX shell scripts like our Porcelain.  I
would suspect that zsh has more "posixly correct" mode, with which
it _can_ run POSIX shell scripts, and I would imagine that this
"$status is an alias $?" business is disabled in that mode?

My quick glance across the codepaths in the callers of this funciton
indicated that it should be safe not using this global variable, so
my answer to my original question was "no there is no significance".
I think we can safely remove any mention of status from this shell
function, i.e. if we remove initial assignment to 0, remove this new
assignment and then remove the "return $status" at the end, the
caller would still be happy.