Subject: Re: [PATCH 02/14] config.c: use kvi for CLI config

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 01 May 2023 13:06:33 +0200

On Fri, Apr 21 2023, Glen Choo via GitGitGadget wrote:

> @@ -682,17 +677,30 @@ static int config_parse_pair(const char *key, const char *value,
>  	if (git_config_parse_key(key, &canonical_name, NULL))
>  		return -1;
>  
> -	ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
> +	ret = (kvi_fn(fn, canonical_name, value, kvi, data) < 0) ? -1 : 0;
>  	free(canonical_name);
>  	return ret;
>  }

This is pre-existing, but I'd much prefer as we're doing more
formalization of this interface if this were just:

	ret = kvi_fn(...);
	...;
	return ret;

I.e. a look at the current code shows us that the API users of
git_config_parse_parameter() are already doing this coercion themselves,
i.e. they only care about "ret < 0".

So let's just hand them the actual return value, rather than doing the
needless coercion.

> @@ -2423,19 +2429,13 @@ static int configset_add_value(struct config_reader *reader,
>  	l_item->e = e;
>  	l_item->value_index = e->value_list.nr - 1;
>  
> -	if (!reader->source)
> -		BUG("configset_add_value has no source");
> -	if (reader->source->name) {
> +	if (reader->source && reader->source->name) {
>  		kv_info->filename = strintern(reader->source->name);
>  		kv_info->linenr = reader->source->linenr;
>  		kv_info->origin_type = reader->source->origin_type;
> -	} else {
> -		/* for values read from `git_config_from_parameters()` */
> -		kv_info->filename = NULL;
> -		kv_info->linenr = -1;
> -		kv_info->origin_type = CONFIG_ORIGIN_CMDLINE;
> -	}
> -	kv_info->scope = reader->parsing_scope;
> +		kv_info->scope = reader->parsing_scope;
> +	} else
> +		kvi_from_param(kv_info);

Missing a {} here.