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, 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.

If we do not go through Perl, we have to set up the git directory and
parse the config in git.c *just* to figure out whether we want to
magically forward difftool to builtin-difftool. That is not only ugly, but
has potential side effects I was not willing to risk.

In any case, this feature flag will be there only for one or two Git for
Windows releases, to give early adopters a chance to send me bug reports
about any regressions.

To be crystal-clear: I never expected this patch to enter git.git.

In that light, I am okay with taking the heat for introducing a temporary,
Git for Windows-only feature flag that is implemented as a "does the file
<xyz> exist?" test.

Ciao,
Dscho