Re: [PATCH v7 5/8] config: add `git_die_config()` to the config-set API

[Junio C Hamano <gitster@pobox.com>]

Tanay Abhra <tanayabh@gmail.com> writes:

> On 8/4/2014 11:37 PM, Junio C Hamano wrote:
>> Tanay Abhra <tanayabh@gmail.com> writes:
>> 
>>> Add `git_die_config` that dies printing the line number and the file name
>>> of the highest priority value for the configuration variable `key`.
>>>
>>> It has usage in non-callback based config value retrieval where we can
>>> raise an error and die if there is a semantic error.
>>> For example,
>>>
>>> 	if (!git_config_get_value(key, &value)) {
>>> 		/* NULL values not allowed */
>>> 		if (!value)
>>> 			git_config_die(key);
>>> 		else
>>> 			/* do work */
>>> 	}
>> 
>> It feels a bit unnatural at the API level that this does not take
>> 'value'; I do understand that it is not a big deal in the error code
>> path to locate again the value from the configuration using the key,
>> but still.
>>
>
> But, we don't have a use for "value" as it is not denoted in the error
> string, that is why I left it out.

That is my point.  Why doesn't the error message talk about what
value the caller found was offensive, and in what way?

>>> +	else
>>> +		die(_("bad config variable '%s' at file line %d in %s"),
>> 
>> At least, quote the last '%s'.
>>
>
> Noted. Thanks.

Actually, "at file line" sounded very strange, at least to me, hence
the suggested reword in the part you did not quote.