Re: [PATCH v3 5/6] config: add `git_die_config()` to the config-set API
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Jul 28, 2014, 11:16 UTC
- Message-ID
- <vpqvbqhu41s.fsf@anie.imag.fr>
- In-Reply-To
- <53D62DBD.9@gmail.com>
Tanay Abhra <tanayabh@gmail.com> writes:
Show 17 quoted lines
> On 7/28/2014 4:25 PM, Ramsay Jones wrote:
>> On 28/07/14 11:33, Tanay Abhra wrote:
>>> + if (!kv_info->linenr)
>>> + die("unable to parse command-line config");
>>> + else
>>> + die("bad config file line %d in %s",kv_info->linenr, kv_info->filename);
>>
>> How about including the 'key' string in the error message?
>> Similar comment applies to an earlier patch in this series.
>>
>
> git_config_get_string & pathname() are already raising a
> config_error_nonbool() with the error message,
> "Missing value for 'key'".
>
> For other cases, I was just following the previous error message,
> but it can be done for the new git_config().It is good to follow the existing in this patch (better code, same behavior), but I agree with Ramsay that including key in the error message (for both branches of the if/then/else) in a further patch would be good, and should be straightforward enough.
-- Matthieu Moy http://www-verimag.imag.fr/~moy/