Re: [PATCH v4 05/10] userdiff: add and use for_each_userdiff_driver()

On Thu, Mar 25 2021, Jeff King wrote:

On Thu, Mar 25, 2021 at 12:05:09AM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

  As an aside, it feels like this is something we ought to be able to
  ask git-config about, rather than having a test-helper. This is
  basically "baked-in" config, and if we represented it as such, and
  parsed it into a struct just like regular config, then probably "git
  config --list --source" could be used to find it (and differentiate it
  from user-provided config). Possible downsides:

    1. Would people find it confusing that "git config --list" suddenly
       gets way bigger? Maybe we'd want an "--include-baked-in" option
       or something.

    2. Is the cost of parsing the config measurably bad? Obviously a
       user could provide the same content and we'd have to parse it,
       but there's a lot more rules here than most users would probably
       provide.

Also:

 3. Only the PATTERNS() macro translates as-is to config syntax. We
    don't have a way to do what IPATTERN() does in the config syntax
    currently.

    We could add a ifuncname and xifuncname or whatever for it I guess,
    but currently the ICASE behavior in the C code is magic.

Good point. IMHO that is something we should consider fixing
independently. It was a mistake to add builtins that couldn't be
replicated via the config (though I notice it happened quite a while
ago, and nobody seems to have cared, so perhaps it isn't that
important).

I'm conspiring to eventually optimistically replace these ERE patterns
with PCRE if we build with that at some point.

Then you could just prefix your pattern with (?i) here in this and other
things that want icase...

I have also wondered if we should just ship a file which could be
installed as /etc/gitconfig.filetypes and include it the stock
/etc/gitconfig. That is effectively the same as "baked in", but
hopefully makes it more clear to users how they can modify things.  But
all of that is somewhat orthogonal to what you're doing here.

In theory I'm with you on that, in practice this is just the sort of
thing that requires opt-in effort from every person packaging git
(installing system-wide-config).

A good number of those people will decide "default config values? In
some ~200 line file included in /etc/gitconfig?? I don't need that! This
is a minimal install!".

And then their web UI that calls "git diff" under the hood won't do the
right thing with a .gitattributes config that expects these built-in
drivers anymore.

But yeah, there's no reason your earlier suggestion of injecting global
defaults into "git config -l" wouldn't work, and for our own C APIs such
as this one to rely on that happening.

It would even make git more discoverable, as all the "this is set to xyz
by default" docs in git-config(1) could become reflective, you could
just run "git config -l --show-origin | grep ^default:" or something to
see all default values.