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

From: Jeff King <[email-masked]>
Date: Thu, 25 Mar 2021 23:27:52 -0400

On Thu, Mar 25, 2021 at 02:26:21AM +0100, Ævar Arnfjörð Bjarmason wrote:

> >>     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 really like PCRE myself, but is it portable/common enough for us to
start using it for baked-in funcname patterns? I sort of assumed there
were exotic platforms where libpcre wouldn't build (or at least it would
be inconvenient or uncommon to have it). And it would be nice to degrade
those gracefully (or at least better than "I guess you don't get any
builtin funcnames. Tough luck").

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

I assumed that we'd install it with "make install", and packaging would
pick it up from there. But you're right that it is likely to create extra
headaches.

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

Yeah, exactly. The discoverability is the real value IMHO. I just worry
about overwhelming the user who runs "git config" with --show-origin. I
guess it could avoid showing baked-in config unless an extra option is
given, but then that makes discoverability slightly harder (though still
better than it is today).

I dunno. This is mostly orthogonal to your patch series, so I don't mind
at all punting on it for now. Though if we _did_ do the baked-in config
then, then a lot of this userdiff code would want to be converted to it.

-Peff