On 7/31/2014 5:08 PM, Matthieu Moy wrote:
> Matthieu Moy <matthieu....@grenoble-inp.fr> writes:
>> Tanay Abhra <tanay...@gmail.com> writes:
>>> On 7/30/2014 7:43 PM, Matthieu Moy wrote:
>>>> * if (!values->items[i].string)
>>>>           config_error_nonbool(
>>>>   => This check could be done once and for all in a function, say
>>>>   git_config_get_value_multi_nonbool, a trivial wrapper around
>>>>   git_config_get_value_multi like
>>>> const struct string_list *git_configset_get_value_multi_nonbool(struct 
>>>> config_set *cs, const char *key)
>>>> {
>>>>    struct string_list l = git_configset_get_value_multi(cs, key);
>>>>         // possibly if(l) depending on the point above.
>>>>    for (i = 0; i < values->nr; i++) {
>>>>            if (!values->items[i].string)
>>>>                    git_config_die(key);
>>>>    }
>>>>    return l;
>>>> }
>>> Not worth it, most the multi value calls do not die on a nonbool.
>> Can you cite some multi-value variables that can be nonbool? I can't
>> find many multi-valued variables, and I can't find any which would allow
>> bool and nonbool.
> Thinking a bit more about it: we actually need more than your patch and
> my code example above to give accurate error messages. Your code gives
> no error message, and mine uses git_config_die(key); which gives the
> line of the _last_ entry, but not necessarily the right line.
> The right line number should be extracted from the info field of the
> string_list. It's not completely trivial, hence I'd rather have a helper
> doing it well in config.c than letting callers re-do the check and
> possibly give wrong line in their error message as I did in my first
> attempt.
> I think you can introduce a helper git_config_die_linenr(key, linenr)
> that displays the right error message. Then git_config_die becomes a
> wrapper around it that does the lookup to find linenr from the hash.
> You already have a duplicate piece of code in your other series:
>                       if (!kv_info->linenr)
>                               die("unable to parse '%s' from command-line 
> config", entry->key);
>                       else
>                               die("bad config variable '%s' at file line %d 
> in %s",
>                                       entry->key,
>                                       kv_info->linenr,
>                                       kv_info->filename);
> That would be the content of git_config_die_linenr().

Thanks. I will add it in the next reroll.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to