Re: [PATCH v4 2/6] config: add new way to pass config via `--config-env`
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Dec 11, 2020, 13:24 UTC
- Message-ID
- <X9NzE5+LNYqG1s+o@ncase>
- In-Reply-To
- <X9D23LQv34A5Q5DC@coredump.intra.peff.net>
On Wed, Dec 09, 2020 at 11:10:04AM -0500, Jeff King wrote:
Show 24 quoted lines
> On Wed, Dec 09, 2020 at 12:52:26PM +0100, Patrick Steinhardt wrote: > > > Co-authored-by: Jeff King <peff@peff.net> > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > In case we want it, this is also: > > Signed-off-by: Jeff King <peff@peff.net> > > > +--config-env=<name>=<envvar>:: > > + Pass a configuration parameter to the command. The <envvar> > > + given will be replaced with the contents of the environment > > + variable of that name. In contrast to `-c`, an envvar must > > + always be given and exist in the environment. Passing an > > + environment variable with empty value will set <name> to the > > + empty string which `git config --type=bool` will convert to > > + `false`. > > I agree with Ævar that we probably should keep an empty variable as the > empty string. I think some options use an empty string to clear a list > (e.g., push.pushOption), and I'm not sure how they'd react to a bool > instead. It would be nice to also have a way to do the implicit-bool > thing, but I don't think it's strictly necessary (it's always correct to > put the string "true" into the variable instead).
I think this is just weirdly worded in the `-c` case, which I mostly copied. We _do_ keep the empty string, which effectively means that `git config --type=bool` will return `false`.
Or do you mean that we should allow `--config-env=foo.bar=`?
> I think we should also document that <envvar> can't contain an "=" sign. > Of course using strrchr() here doesn't help much with just this patch, > because we flatten the string before stuffing it into > $GIT_CONFIG_PARAMETERS, so the reading side would mis-parse it.
Makes sense.
Patrick
Show 15 quoted lines
> But here's a fix for that. I built it on top of your whole series, since > you touched some of the related functions, but it could easily be > rebased onto just this part. > > [1/3]: quote: make sq_dequote_step() a public function > [2/3]: config: parse more robust format in GIT_CONFIG_PARAMETERS > [3/3]: config: store "git -c" variables using more robust format > > config.c | 118 +++++++++++++++++++++++++++++++++++++--------- > quote.c | 15 ++++-- > quote.h | 18 ++++++- > t/t1300-config.sh | 60 +++++++++++++++++++++++ > 4 files changed, 183 insertions(+), 28 deletions(-) > > -Peff