public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

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 v6 6/7] config: add `git_die_config()` to the config-set API
Date: Fri, 01 Aug 2014 11:22:57 +0200	[thread overview]
Message-ID: <vpqlhr836oe.fsf@anie.imag.fr> (raw)
In-Reply-To: <53DB582F.8040501@gmail.com> (Tanay Abhra's message of "Fri, 01 Aug 2014 14:34:47 +0530")

Tanay Abhra <tanayabh@gmail•com> writes:

> Thanks, I am sending your version with the reroll.
> Also, for clarity the multi value use case would look like,
>
> 	struct key_value_info *kv_info;
> 	if (load_config_refs) {
> 		values = git_config_get_value_multi("notes.displayref");
> 		if (values) {
> 			for (i = 0; i < values->nr; i++) {
> 				if (!values->items[i].string) {
> 					config_error_nonbool("notes.displayref");
> 					kv_info = values->items[i].util;
> 					git_die_config_linenr("notes.displayref",
> 							      kv_info->filename,
> 							      kv_info->linenr);
> 				}
> 				else
> 					string_list_add_refs_by_glob(&display_notes_refs,
> 								     values->items[i].string);
> 			}
> 		}
> 	}
>
> with my function it would have looked like,
>
> 	if (load_config_refs) {
> 		values = git_config_get_value_multi("notes.displayref");
> 		if (values) {
> 			for (i = 0; i < values->nr; i++) {
> 				if (!values->items[i].string) {
> 					config_error_nonbool("notes.displayref");
> 					git_die_config_exact("notes.displayref", values->items[i].string);
> 				}
> 				else
> 					string_list_add_refs_by_glob(&display_notes_refs,
> 								     values->items[i].string);
> 			}
> 		}
> 	}

I still think that checking for non-null values should be done in a
helper in config.c, and then the code would look like

 	if (load_config_refs) {
 		values = git_config_get_value_multi_nonbool("notes.displayref");
 		if (values)
 			for (i = 0; i < values->nr; i++)
				string_list_add_refs_by_glob(&display_notes_refs,
							     values->items[i].string);
 	}

The same helper could at least be used for "branch.<remote>.merge".

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

next prev parent reply	other threads:[~2014-08-01  9:23 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
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 [this message]
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=vpqlhr836oe.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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox