Re: [RFC PATCH] userdiff: ship built-in driver config file

[Jeff King <peff@peff.net>]

On Tue, Jun 18, 2019 at 10:32:47PM +0200, Johannes Sixt wrote:

> Am 17.06.19 um 18:54 schrieb Boxuan Li:
> > The userdiff.c has been rewritten to avoid hard-coded built-in
> > driver patterns. Now we ship
> > $(sharedir)/git-core/templates/userdiff that can be read using
> > git_config_from_file() interface, using a very narrow callback
> > function that understands only diff.*.xfuncname,
> > diff.*.wordregex, and diff.*.regIcase.
> > 
> > Signed-off-by: Boxuan Li <liboxuan@connect.hku.hk>
> > ---
> > A few notes and questions:
> > 1. In [diff "tex"] section, \x80 and \xff cannot be parsed by git config parser.
> > I have no idea why this is happening. I changed them to \\x80 and \\xff as a workaround, which
> > resulted in t4034 failure (See https://travis-ci.org/li-boxuan/git/jobs/546729906#L4679).
> 
> I guess, the idea is to catch bytes of UTF-8 encoded characters as
> regular words.
> 
> The problem is to write such bytes literally into a git-config file and
> still keep the file editable in a portable. Perhaps it is necessary to
> declare the file as CP1252 encoded via .gitattributes, write that part
> of the regexp as [a-zA-Z0-9€-þ], and hope that your text editor writes
> the file acutally as CP1252. ISO8859-1 does not work because \x80 is not
> occupied.

We don't allow octal or hex escapes in config values, though we do allow
common C ones like "\n". Maybe we should support them.

I didn't check whether we actually want the raw bytes here ourselves,
though, or are happy making sure the backslashed forms make it into the
regex parser (it _seems_ like the latter would be what we want, but that
does mean escaping the backslashes so they make it through the config
parser literally).

> > diff --git a/templates/this--userdiff b/templates/this--userdiff
> > new file mode 100644
> > index 0000000000..85114a7229
> > --- /dev/null
> > +++ b/templates/this--userdiff
> 
> Why place this file in .git? To have per-repository diff drivers, we can
> already specify them via 'git config'. This file should be installed in
> the system.

I think it _could_ actually just be part of the system /etc/gitconfig,
though it is kind of big, and Git has a tendency to parse the config
more than necessary. I wonder if would add a noticeable slowdown.

-Peff