Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin

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

Jeff King <peff@peff.net> writes:

> On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
>
>> > If you want to control it from outside the test script, you'd need
>> > something like:
>> > 
>> >   if test "$GIT_TEST_DIFFTOOL" = "builtin"
>> 
>> That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
>> not work. My name is arguably more correct (see also Jakub's note about
>> "naming is hard"), but yours works because there is a "TEST" substring in
>> it.
>
> Yes. You are free to add an exception to the env list in test-lib.sh,
> but we usually use GIT_TEST_* to avoid having to do so.

Perhaps

 - The switch between "do I use builtin, or scripted?" mechanism in
   1/2 can look at an environment (just like the old "am" rewrite
   series did), instead of configuration.  This would make the code
   a lot more simppler (you do not have to worry about the
   interaction between "setup" and .git/config).

 - That environment variable can be named GIT_TEST_BUILTIN_DIFFTOOL;
   after all, people are opting into helping to test the new shiny
   to make/prove it ready sooner.

 - The bulk of the existing test for difftool can be moved to a
   dot-included file (in a way similar to t/annotate-tests are
   usable to test both annotate and blame-imitating-annotate).
   Existing PERL prerequisites can all be lost.

 - Two tests can include that dot-included file; one would
   explicitly unset that environment (and gives up without PERL
   prerequisite), while the other explicitly sets it.