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

2 messages, 2 authors, 2021-09-24 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-09-24 21:28:38

Jeff King [off-list ref] writes:
I had thought to extend with the operator, like:

  # equality
  [includeIf "env:FOO==value"]
  # regex
  [includeIf "env:FOO=~v[a]l"]
Yup, that matched my aesthetics better ;-)
But as you note, "=" is somewhat problematic, and without that we can't
use the "usual" operators. Plus there's no usual operator for globbing. ;)
So embedding it in the name is fine by me (and mostly a bikeshed thing
anyway).
Perhaps.  I am not sure if we deeply care about "git -c var=val" in
this case, especially since this is part of includeif, though.  It
may be more important to keep the syntax useful and extensible for
everyday use than for one-off "git -c" testing.
I agree we don't really need a "/i" variant here.
Case insensitive environment variable names, no, but case
insensitive matching of values, maybe?  But I'd be happy to see us
start very minimally (even just envEQ alone without any other
frills, or optionally envNE to negate it, would be fine by me).
Should we allow whitespace around key names and values? E.g.:

  [includeIf "env: FOO: bar"]

is IMHO more readable (even more so if we had infix operators like
"==").
This asserts what? FOO=" bar"?

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

From: Jeff King <hidden>
Date: 2021-09-24 21:59:29

On Fri, Sep 24, 2021 at 02:28:29PM -0700, Junio C Hamano wrote:
quoted
But as you note, "=" is somewhat problematic, and without that we can't
use the "usual" operators. Plus there's no usual operator for globbing. ;)
So embedding it in the name is fine by me (and mostly a bikeshed thing
anyway).
Perhaps.  I am not sure if we deeply care about "git -c var=val" in
this case, especially since this is part of includeif, though.  It
may be more important to keep the syntax useful and extensible for
everyday use than for one-off "git -c" testing.
Yeah, see my comments later in that mail. :)
quoted
I agree we don't really need a "/i" variant here.
Case insensitive environment variable names, no, but case
insensitive matching of values, maybe?  But I'd be happy to see us
start very minimally (even just envEQ alone without any other
frills, or optionally envNE to negate it, would be fine by me).
Yeah, as long as we leave the door open syntactically, I think it is OK.
quoted
Should we allow whitespace around key names and values? E.g.:

  [includeIf "env: FOO: bar"]

is IMHO more readable (even more so if we had infix operators like
"==").
This asserts what? FOO=" bar"?
Whoops, that should have been "envIs", asserting that $FOO contains
"bar".

As I said, I think it matters more with the infix operators, as:

  [includeIf "env:FOO == bar"]

is more readable than:

  [includeIf "env:FOO==bar"]

But I do think:

  [includeIf "envIs:FOO:bar"]

is harder to read than even:

  [includeIf "envIs:FOO: bar"]

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help