From: David Aguilar <davvid@gmail.com>
Date: Wed, 26 Sep 2012 18:31:45 GMT
Subject: Re: [PATCH] mergetool--lib: Allow custom commands to override built-ins
Message-ID: <CAJDDKr4OkvAvRQFjp0NuzYkE2DSdvXh1vvTdKeaCq+bvQA4kuQ@mail.gmail.com>
In-Reply-To: <7vtxukubds.fsf@alter.siamese.dyndns.org>

On Wed, Sep 26, 2012 at 7:06 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Hi David,
>>
>> David Aguilar wrote:
>>>  diff_cmd () {
>>> -       merge_tool_cmd="$(get_merge_tool_cmd "$1")"
>>> -       if test -z "$merge_tool_cmd"
>>> -       then
>>> -               status=1
>>> -               break
>>> -       fi
>>> -       ( eval $merge_tool_cmd )
>>> -       status=$?
>>> +       status=1
>>>         return $status
>>>  }
>>
>> Nit: Why not return 1, instead of setting $status and returning it?
>
> Perhaps because the caller "run_merge_tool" pays attention to
> $status that is a global variable?
>
> Have you traced the call chain?

Exactly.  I would like to eliminate globals whenever possible,
but this particular topic involved refactoring which aimed to keep
existing behavior w.r.t these variables unchanged.
-- 
David