Re: [PATCH] mergetool--lib: Allow custom commands to override built-ins

From: David Aguilar <davvid@gmail.com>
Date: Sep 26, 2012, 18:31 UTC
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:

Show 24 quoted lines

> 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