Re: [PATCH V4] config: add --expiry-date

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 18, 2017, 03:37 UTC
Message-ID: <xmqq8tf4qmu8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171118022727.30179-1-hsed@unimetic.com>

hsed@unimetic.com writes:

Show 18 quoted lines

> diff --git a/config.c b/config.c
> index 903abf953..64f8aa42b 100644
> --- a/config.c
> +++ b/config.c
> @@ -990,6 +990,16 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
>  	return 0;
>  }
>  
> +int git_config_expiry_date(timestamp_t *timestamp, const char *var, const char *value)
> +{
> +	if (!value)
> +		return config_error_nonbool(var);
> +	if (parse_expiry_date(value, timestamp))
> +		return error(_("'%s' for '%s' is not a valid timestamp"),
> +			     value, var);
> +	return 0;
> +}
> +

I think this is more correct even within the context of this function than dying, which suggests the need for a slightly related (which is not within the scope of this change) clean-up within this file as a #leftoverbits task. I think dying in these value parsers goes against the point of having die_on_error bit in the config-source structure; Heiko and Peff CC'ed for b2dc0945 ("do not die when error in config parsing of buf occurs", 2013-07-12).

Thanks; will queue.