git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, The Grey Wolf <greywolf@starwolf.com>,
	"Randall S . Becker" <rsbecker@nexbridge.com>
Subject: Re: [PATCH] config: add an includeIf.env{Exists,Bool,Is,Match}
Date: Tue, 28 Sep 2021 21:28:25 +0200	[thread overview]
Message-ID: <87bl4cwkr8.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <YVKrRooSIN7OeLy9@coredump.intra.peff.net>


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/xmqqo88eq8um.fsf@gitster.g/):

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

  reply	other threads:[~2021-09-28 19:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23  5:21 ANSI sequences produced on non-ANSI terminal The Grey Wolf
2021-09-23 21:20 ` Jeff King
2021-09-23 21:54   ` Junio C Hamano
2021-09-23 22:04     ` Randall S. Becker
2021-09-25  6:45       ` Kevin Daudt
2021-09-24  0:58   ` [PATCH] config: add an includeIf.env{Exists,Bool,Is,Match} Ævar Arnfjörð Bjarmason
2021-09-24 21:07     ` Jeff King
2021-09-24 21:28       ` Junio C Hamano
2021-09-24 21:59         ` Jeff King
2021-09-27 16:30           ` Junio C Hamano
2021-09-27 20:15             ` Jeff King
2021-09-27 20:53               ` Randall S. Becker
2021-09-27 21:37                 ` Jeff King
2021-09-27 21:56                   ` Randall S. Becker
2021-09-27 23:52               ` Ævar Arnfjörð Bjarmason
2021-09-28  0:41                 ` Jeff King
2021-09-28  2:42                   ` Ævar Arnfjörð Bjarmason
2021-09-28  5:42                     ` Jeff King
2021-09-28 19:28                       ` Ævar Arnfjörð Bjarmason [this message]
2021-09-28  0:24               ` Junio C Hamano
2021-09-24 23:57   ` ANSI sequences produced on non-ANSI terminal Greywolf
2021-09-25  5:49     ` Jeff King
2021-10-01 23:17       ` Greywolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bl4cwkr8.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=greywolf@starwolf.com \
    --cc=peff@peff.net \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).