From: Jonathan Tan <jonathantanmy@google.com>
Date: Fri, 23 Jun 2023 20:35:44 GMT
Subject: Re: [PATCH v3 07/12] config.c: pass ctx with CLI config
Message-ID: <20230623203544.3255750-1-jonathantanmy@google.com>
In-Reply-To: <1fb1708bbd967ed841fcb3d96ea0d1f6b880c60e.1687290233.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> @@ -2465,16 +2488,10 @@ 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) {
> -		kvi_from_source(reader->source, current_config_scope(), kv_info);
> +		kvi_from_source(reader->source, kvi_p->scope, kv_info);
>  	} 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;
> +		kvi_from_param(kv_info);
>  	}
>  	si->util = kv_info;
>  

Any reason to remove the "if (!reader->source)" guard? I don't think
this patch does anything to ensure that reader is present. We can
probably remove this once "reader" is removed.

The rest of the patch looks fine (and all patches prior look fine too.