All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Tanay Abhra <tanayabh@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>
Subject: Re: [PATCH v7 5/8] config: add `git_die_config()` to the config-set API
Date: Tue, 05 Aug 2014 17:15:27 +0200	[thread overview]
Message-ID: <vpqiom72cj4.fsf@anie.imag.fr> (raw)
In-Reply-To: <53E0F057.1040706@gmail.com> (Tanay Abhra's message of "Tue, 05 Aug 2014 20:25:19 +0530")

Tanay Abhra <tanayabh@gmail.com> writes:

> Matthieu, I have finished the new version, but instead of flooding the mailing list with
> a series again, I wanted to confirm if the new git_config_die() is alright.
>
> 	NORETURN __attribute__((format(printf, 2, 3)))
> 	void git_die_config(const char *key, const char *err, ...)
> 	{
> 		const struct string_list *values;
> 		struct key_value_info *kv_info;
>
> 		if (err) {
> 			va_list params;
> 			va_start(params, err);
> 			vreportf("error: ", err, params);
> 			va_end(params);
> 		}
> 		values = git_config_get_value_multi(key);
> 		kv_info = values->items[values->nr - 1].util;
> 		git_die_config_linenr(key, kv_info->filename, kv_info->linenr);
> 	}
>
> Currently works like the old git_config() error reporting path. If err is set to "NULL",
> it would print no error message and just the die message. If given something like,
>
> 	 git_config_die(key, "value '%s' is not allowed", value);
>
> it would print,
>
> 	error: value '3' is not allowed
> 	fatal: bad config variable 'core.frotz' at file line 15 in .git/config

That seems to be a good step forward.

I think we would also want to improve the error message, but that
shouldn't block your series from inclusion: we can do that later without
API change.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2014-08-05 15:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-01 17:05 [PATCH v7 0/8] Rewrite `git_config()` using config-set API Tanay Abhra
2014-08-01 17:05 ` [PATCH v7 1/8] config.c: mark error and warnings strings for translation Tanay Abhra
2014-08-01 17:05 ` [PATCH v7 2/8] config.c: fix accuracy of line number in errors Tanay Abhra
2014-08-04 13:41   ` Matthieu Moy
2014-08-01 17:05 ` [PATCH v7 3/8] add line number and file name info to `config_set` Tanay Abhra
2014-08-01 17:05 ` [PATCH v7 4/8] change `git_config()` return value to void Tanay Abhra
2014-08-01 17:05 ` [PATCH v7 5/8] config: add `git_die_config()` to the config-set API Tanay Abhra
2014-08-04 18:07   ` Junio C Hamano
2014-08-04 18:55     ` Tanay Abhra
2014-08-04 20:04       ` Matthieu Moy
2014-08-05 14:55         ` Tanay Abhra
2014-08-05 15:15           ` Matthieu Moy [this message]
2014-08-05 16:42             ` Junio C Hamano
2014-08-04 20:52       ` Junio C Hamano
2014-08-01 17:05 ` [PATCH v7 6/8] rewrite git_config() to use " Tanay Abhra
2014-08-01 17:05 ` [PATCH v7 7/8] add a test for semantic errors in config files Tanay Abhra
2014-08-04 18:13   ` Junio C Hamano
2014-08-01 17:05 ` [PATCH v7 8/8] add tests for `git_config_get_string_const()` Tanay Abhra
2014-08-04 13:43 ` [PATCH v7 0/8] 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=vpqiom72cj4.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tanayabh@gmail.com \
    /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.