From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Fri, 01 Aug 2014 09:22:57 GMT
Subject: Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
Message-ID: <vpqlhr836oe.fsf@anie.imag.fr>
In-Reply-To: <53DB582F.8040501@gmail.com>

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/