Tanay Abhra <tanayabh@xxxxxxxxx> writes: > On 7/31/2014 9:25 PM, Matthieu Moy wrote: >> Tanay Abhra <tanayabh@xxxxxxxxx> 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/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html