All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tanay Abhra <tanayabh@gmail.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>
Subject: Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
Date: Thu, 31 Jul 2014 22:14:49 +0530	[thread overview]
Message-ID: <53DA7281.8060403@gmail.com> (raw)
In-Reply-To: <vpqr411h69h.fsf@anie.imag.fr>



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. git_die_config() will be a wrapper over it, supplying the last value.
It will also replace the redundant code in git_config_raw() and help in the
multi value case. If the idea sounds okay, I will send a reroll.

  reply	other threads:[~2014-07-31 16:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-31 15:47 [PATCH v6 0/7] Rewrite `git_config()` using config-set API Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 1/7] config.c: fix accuracy of line number in errors Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 2/7] add line number and file name info to `config_set` Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 3/7] change `git_config()` return value to void Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 4/7] rewrite git_config() to use the config-set API Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 5/7] add a test for semantic errors in config files Tanay Abhra
2014-07-31 15:47 ` [PATCH v6 6/7] config: add `git_die_config()` to the config-set API Tanay Abhra
2014-07-31 15:55   ` Matthieu Moy
2014-07-31 16:44     ` Tanay Abhra [this message]
2014-07-31 16:52       ` Matthieu Moy
2014-07-31 17:25         ` Tanay Abhra
2014-07-31 18:09           ` Matthieu Moy
2014-07-31 18:26             ` Tanay Abhra
2014-07-31 18:41               ` Matthieu Moy
2014-07-31 21:17                 ` Junio C Hamano
2014-08-01  8:33                   ` Matthieu Moy
2014-08-01  9:04                 ` Tanay Abhra
2014-08-01  9:22                   ` Matthieu Moy
2014-07-31 15:47 ` [PATCH v6 7/7] add tests for `git_config_get_string_const()` Tanay Abhra
2014-07-31 15:56 ` [PATCH v6 0/7] Rewrite `git_config()` using config-set API Matthieu Moy

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=53DA7281.8060403@gmail.com \
    --to=tanayabh@gmail.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.