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

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

Hi Peff,

On Thu, 24 Nov 2016, Jeff King wrote:

> On Thu, Nov 24, 2016 at 10:56:23PM +0100, Johannes Schindelin wrote:
> 
> > > I think it would probably be OK to ship with that caveat (people would
> > > probably use --global config, or "git -c" for a quick override), but if
> > > you really wanted to address it, you can do something like what
> > > pager.c:read_early_config() does.
> > 
> > The config setting is already overkill (and does even make something much
> > harder than before: running tests with the builtin difftool used to be as
> > simply as `touch use-builtin-difftool && make -C t t7800-difftool.sh, now
> > I have to edit t7800-difftool.sh to configure difftool.useBuiltin, and
> > without the repo-level config even that would not be working).
> > 
> > Imitating read_early_config() would be overkill deluxe.
> 
> I would have expected it to just be a build-time flag, like:
> 
>   make BUILTIN_DIFFTOOL=Yes test

That works for Git developers.

I want to let as many users as possible test the builtin difftool.
Hopefully a lot more users than there are Git developers.

Which means that I need a feature flag in production code, not a build
time flag.

> I'm happy with pretty much anything under the reasoning of "this does not
> matter much because it is going away soon".

Yeah, well, I am more happy with anything along the lines of David's
review, pointing out flaws in the current revision of the builtin difftool
before it bites users ;-)

Ciao,
Dscho