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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Dennis,

On Wed, 23 Nov 2016, Johannes Schindelin wrote:

> On Wed, 23 Nov 2016, Dennis Kaarsemaker wrote:
> 
> > On Tue, 2016-11-22 at 18:01 +0100, Johannes Schindelin wrote:
> > > The original idea was to use an environment variable
> > > GIT_USE_BUILTIN_DIFFTOOL, but the test suite resets those variables, and
> > > we do want to use that feature flag to run the tests with, and without,
> > > the feature flag.
> > > 
> > > Besides, the plan is to add an opt-in flag in Git for Windows'
> > > installer. If we implemented the feature flag as an environment
> > > variable, we would have to modify the user's environment, in order to
> > > make the builtin difftool the default when called from Git Bash, Git CMD
> > > or third-party tools.
> > 
> > Why is this not a normal configuration variable (as in git config
> > difftool.builtin true or something)? It doesn't make much sense to me
> > to introduce a way of configuring git by introducing magic files, when
> > a normal configuration variable would do just fine, and the GfW
> > installer can also set such variables, like it does for the crlf config
> > I believe.
> 
> I considered that. Adding a config setting would mean we simply test for
> it in git-difftool.perl and call the builtin if the setting is active,
> right?
> 
> The downside is that we actually *do* go through Perl to do that. Only to
> go back to a builtin. Which is exactly the thing I intended to avoid.

Okay, I reconsidered. Junio's comment about how git-am did it made me
rethink the issue: I need not keep the name "difftool" for the script. So
what I do now is rename the Perl script to git-legacy-difftool and always
read the config in the builtin difftool, handing off to the legacy
difftool unless core.useBuiltinDifftool=true.

This is an easy way to do it, and a portable and clean blueprint for
similar feature-flags in the future.

Ciao,
Johannes