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