Subject: Re: [PATCH] config: add an includeIf.env{Exists,Bool,Is,Match}

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 28 Sep 2021 21:28:25 +0200

On Tue, Sep 28 2021, Jeff King wrote:

> On Tue, Sep 28, 2021 at 04:42:51AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> > A perhaps more subtle but less awkward to type version is to just
>> > require two arguments, like:
>> >
>> >   git --config <key> <value> ...
>> 
>> I suppose --config would work like that, you can'd to it with "-c". I
>> think it's more confusing to have a "-c" and "--config" which unlike
>> most other things don't follow the obvious long and short option names
>> working the same way.
>
> Yeah, probably "--config-pair" or something might be less confusing.
> Anyway...

*nod*

>> > but I'd just as soon continue to leave it un-implemented if nobody has
>> > actually needed it in practice.
>> 
>> *nod*. I do think it's bad design to introduce an "env" inclusion
>> feature that relies on "=" though while we don't have something like
>> that, i.e.
>> 
>> I think we should probably not add that --config-{key,value}, but
>> avoiding the arbitrary limitation of not being able to specify certain
>> config keys seems prudent in that case, and since the "=" v.s. ":" is
>> only an aesthetic preference I think being able to compose things
>> without limitations wins out.
>
> I don't really agree with that. Whatever syntax we use now, we'll be
> stuck with forever. It seems a shame to predicate that choice only on
> the "-c doesn't support =" thing that nobody has actually run across in
> practice (and I don't think is something people will run into with
> this).

Yeah, anyway. I don't care much either way, and not enough to drive this
forward. I.e. it seemed like an easy thing to hack up, but if anyone
else is interested in driving it forward...

>> We do have the "=" key limitation now, but I don't think it's there for
>> any key we currently define, except things like "url.<base>.insteadOf"
>> if the "<base> has a "=" in it (and maybe just that one).
>
> It's really a potential problem for any 3-level config key. So urls,
> branch names, remote names, various tool names, filter/diff drivers,
> existing includeIf conditions. This might be the first one where we
> really _encourage_ the use of "=" signs, but it still strikes me as
> weird that you'd want to do so on the command-line in practice.

Just for future reference:

I think given the discussion in the thread, and particularly if we're
going to have some regex syntax for these keys that the artificial
straitjacket of putting this all in one config key is something we
should just do away with.

I.e. I'm not proposing a *specific* schema other than noting that
there's no law that forces us to take say Junio's (in
https://lore.kernel.org/git/[email-masked]/):

        [includeIf "env:PATH ~= \"(:|^)/usr/bin(:|$)\""]

Over say:

    [includeCondition]
        type = envRegex
        envVariable = PATH
        envRegex = "(:|^)/usr/bin(:|$)"
        path = ~/.gitconfig.d/env-stuff.cfg

Or whatever, i.e. the state machine of seeing an "includeCondition" in
the config's event parser, and then erroring unless the next N config
keys satisfy some mandatory minimum set of config keys is rather simple.

We could then make any such syntax optional for existing constructs,
i.e. you could write:

    [includeIf "gitdir:/path/to/group/"]
    path = /path/to/foo.inc

As:

    [includeCondition]
        type = gitdir
        path = /path/to/foo.inc
        gitdirPath = /path/to/group/

Or something. And say add "includeCondition.negated = true" to that for
a "!=" match.

The shorthand syntax could then be omitted for anything new if it's
deemed too gnarly to represent it all in one key.

The key names & schema is something I came up with offhand, please don't
read too much into it. The point is that we don't need to make it all
one key.

That approach also fits nicely in with the rest of the config framework,
i.e. you can incrementally edit things using "git config" options, and
we could add a "--type regex" or whatever which we could then
set/validate say the "includeCondition.envRegex" key with.