Re: [PATCH 2/2] git: support separate arg for `--config-env`'s value

From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] git: support separate arg for `--config-env`'s value
Date: Fri, 23 Apr 2021 06:07:35 -0400	[thread overview]
Message-ID: <YIKcZxEDsG7qsE4G@coredump.intra.peff.net> (raw)
In-Reply-To: <874kg14490.fsf@evledraar.gmail.com>

On Tue, Apr 20, 2021 at 12:52:43PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > Mitigate this inconsistency by accepting both syntaxes and add tests to
> > verify both work.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> This whole series LGTM.

Me too. Thanks for finding it (and Patrick for fixing it).

> > +		} else if (!strcmp(cmd, "--config-env")) {
> > +			if (*argc < 2) {
> > +				fprintf(stderr, _("no config key given for --config-env\n" ));
> 
> 
> I found this use of fprintf() slightly odd, why not error(), but then
> went back and read the function and saw that it has N number of
> "fprintf(stderr, [...])" already.
> 
> That could probably all be converted to error(), but better to be
> consistent for now.

Yep. Likewise the weird extra space at the end of the string.

-Peff