From: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>
To: Tanay Abhra <tanayabh@gmail•com>
Cc: git@vger•kernel.org, Ramkumar Ramachandra <artagnon@gmail•com>
Subject: Re: [PATCH v4 2/5] notes.c: replace `git_config()` with `git_config_get_value_multi()`
Date: Thu, 31 Jul 2014 13:38:56 +0200 [thread overview]
Message-ID: <vpqmwbpdagf.fsf@anie.imag.fr> (raw)
In-Reply-To: <vpqsiliol1o.fsf@anie.imag.fr> (Matthieu Moy's message of "Wed, 30 Jul 2014 18:42:27 +0200")
Matthieu Moy <Matthieu.Moy@grenoble-inp•fr> writes:
> Tanay Abhra <tanayabh@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().
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
next prev parent reply other threads:[~2014-07-31 11:39 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-30 13:39 [PATCH v4 0/5] git_config callers rewritten with the new config cache API Tanay Abhra
2014-07-30 13:39 ` [PATCH v4 1/5] pager.c: replace `git_config()` with `git_config_get_value()` Tanay Abhra
2014-07-30 13:39 ` [PATCH v4 2/5] notes.c: replace `git_config()` with `git_config_get_value_multi()` Tanay Abhra
2014-07-30 14:13 ` Matthieu Moy
2014-07-30 14:40 ` Tanay Abhra
2014-07-30 16:42 ` Matthieu Moy
2014-07-31 11:38 ` Matthieu Moy [this message]
2014-07-31 12:13 ` Tanay Abhra
2014-07-30 13:39 ` [PATCH v4 3/5] imap-send.c: replace `git_config()` with `git_config_get_*()` family Tanay Abhra
2014-07-30 13:39 ` [PATCH v4 4/5] branch.c: replace `git_config()` with `git_config_get_string() Tanay Abhra
2014-07-30 16:23 ` Matthieu Moy
2014-07-30 13:39 ` [PATCH v4 5/5] alias.c: replace `git_config()` with `git_config_get_string()` Tanay Abhra
2014-07-30 13:46 ` [PATCH v4 0/5] git_config callers rewritten with the new config cache API Matthieu Moy
2014-07-30 14:03 ` Tanay Abhra
2014-07-30 16:45 ` Matthieu Moy
2014-07-31 11:37 ` Ramsay Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=vpqmwbpdagf.fsf@anie.imag.fr \
--to=matthieu.moy@grenoble-inp$(echo .)fr \
--cc=artagnon@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=tanayabh@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox