Re: [PATCH 2/2] difftool: add a feature flag for the builtin vs scripted version

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 23, 2016, 20:04 UTC
Message-ID: <xmqq8tsam08n.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1611232051420.3746@virtualbox>

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

Show 25 quoted lines

> Hi Junio,
>
> On Wed, 23 Nov 2016, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > Can't you route the control upon seeing "git difftool" to your
>> > experimental "C" difftool and check the configuration there?  Then
>> > you can decide to run_command() a non-builtin one depending what the
>> > configuration says---that way, you would incur cost of spawning Perl
>> > only when you need it, no?
>> 
>> FWIW, the approach taken by 73c2779f42 ("builtin-am: implement
>> skeletal builtin am", 2015-08-04) is what I had in mind when I wrote
>> the above.
>
> Maybe that worked back then. But I doubt it, because checking out that
> revision, I get this "warning":
>
> Makefile:1732: warning: overriding recipe for target 'git-am'
> Makefile:1696: warning: ignoring old recipe for target 'git-am'
>
> Seems like a matter of luck whether the `make` executable you happen to
> use guesses what we want: to munge git-am.sh into git-am, as opposed to
> hard-linking git to git-am.

You do not need to keep two copies of "git-cmd", though. commands[] table can have an entry "difftool" that points at cmd_difftool(), which switches between run_command("difftool-scripted") or makes a function call to a static difftool_builtin() that you wrote in 1/2.