Re: [PATCH v4 3/4] config: correct bad boolean env value error message

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 29, 2023, 23:03 UTC
Message-ID: <xmqqa5t4inu7.fsf@gitster.g>
In-Reply-To: <e4c20a81f93e2a4e2ad65920920ead690b162e25.1696021277.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 17 quoted lines

> From: Calvin Wan <calvinwan@google.com>
>
> An incorrectly defined boolean environment value would result in the
> following error message:
>
> bad boolean config value '%s' for '%s'
>
> This is a misnomer since environment value != config value. Instead of
> calling git_config_bool() to parse the environment value, mimic the
> functionality inside of git_config_bool() but with the correct error
> message.
>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  config.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Makes sense.

I briefly wondered if there are ways to share more code, but this seems to be the best we can do. The duplication is not too bad to begin with anyway.

Looking good.  Will queue.

Show 20 quoted lines

> diff --git a/config.c b/config.c
> index 3846a37be9..7dde0aaa02 100644
> --- a/config.c
> +++ b/config.c
> @@ -2133,7 +2133,14 @@ void git_global_config(char **user_out, char **xdg_out)
>  int git_env_bool(const char *k, int def)
>  {
>  	const char *v = getenv(k);
> -	return v ? git_config_bool(k, v) : def;
> +	int val;
> +	if (!v)
> +		return def;
> +	val = git_parse_maybe_bool(v);
> +	if (val < 0)
> +		die(_("bad boolean environment value '%s' for '%s'"),
> +		    v, k);
> +	return val;
>  }
>  
>  /*