Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Tanay Abhra <tanayabh@gmail.com> writes:

> On 7/31/2014 9:25 PM, Matthieu Moy wrote:
>> Tanay Abhra <tanayabh@gmail.com> writes:
>> 
>>> +void git_die_config(const char *key)
>>> +{
>>> +	const struct string_list *values;
>>> +	struct key_value_info *kv_info;
>>> +	values = git_config_get_value_multi(key);
>>> +	kv_info = values->items[values->nr - 1].util;
>>> +	if (!kv_info->linenr)
>>> +		die(_("unable to parse '%s' from command-line config"), key);
>>> +	else
>>> +		die(_("bad config variable '%s' at file line %d in %s"),
>>> +			key,
>>> +			kv_info->linenr,
>>> +			kv_info->filename);
>>> + }
>> 
>> Extra whitespace before }.
>> 
>> Also, didn't we agree that it was a good thing to factor this
>> if/then/else into a helper function?
>> 
>
> I have been thinking about it, wouldn't it be better to give users
> a function like,
>
> git_config_die_exact(key, value);
>
> where user supplies key & value both and it would print the correct message based
> on that.

I suggested git_config_die_linenr(key, linenr), and I now realize it
should take the value too.

You're suggesting git_config_die_exact(key, value). Is it a typo that
you forgot the line number, or is it intentional? If intentional, I
don't think it solves your issue:

[section]
   key
   key

There are two errors in this file, and you need to provide a line
number. key and value alone do not allow you to know which line the
error is. You can use a convention like "complain on the first value
equal to the argument", but I'm not sure that would always work. And
that seems backward to me to reconstruct the line number since the
function can be called from places where the line number is already
known (while iterating over the string_list for example).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/