Re: [PATCH v2 1/3] config: add git_config_append_parameter()

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 7, 2026, 08:14 UTC
Message-ID: <ap5yT0sOtLQQa4AY@pks.im>
In-Reply-To: <ef8087e80db18c511e31dd9c7dd6ea3d57dd543c.1788537086.git.gitgitgadget@gmail.com>

On Fri, Sep 04, 2026 at 03:51:24PM +0000, Thomas Bachem via GitGitGadget wrote:

Show 6 quoted lines

> From: Thomas Bachem <mail@thomasbachem.com>
> 
> Split the part of git_config_push_split_parameter() that formats one
> GIT_CONFIG_PARAMETERS entry into a helper that appends it to a strbuf,
> so that a caller can build a value for a child's environment without
> knowing the quoting. The sequencer is about to do that.

Readers who don't have any context around GIT_CONFIG_PARAMETERS and what it does will have a bit of a hard time making much sense of this, I think. It usually helps to give a sentence or two explaining what the infra even does, and what this quoting looks like.

Show 16 quoted lines

> Assisted-by: Claude Fable 5.1
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
> ---
>  config.c | 20 +++++++++++++-------
>  config.h | 10 ++++++++++
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/config.c b/config.c
> index d9019e7e6c..e0bb29b53d 100644
> --- a/config.c
> +++ b/config.c
> @@ -450,18 +450,24 @@ static int git_config_include(const char *var, const char *value,
>  	return ret;
>  }
>  
> +void git_config_append_parameter(struct strbuf *env, const char *key,

Nit: callling this `env` assumes a bit too much about what this buffer
is going to be used for. I'd have called it just `buf`.

Show 25 quoted lines

> diff --git a/config.h b/config.h
> index b66dd08007..fcf48f6245 100644
> --- a/config.h
> +++ b/config.h
> @@ -22,6 +22,7 @@
>   */
>  
>  struct object_id;
> +struct strbuf;
>  
>  /* git_config_parse_key() returns these negated: */
>  #define CONFIG_INVALID_KEY 1
> @@ -186,6 +187,15 @@ int git_config_from_blob_oid(config_fn_t fn, const char *name,
>  			     enum config_scope scope);
>  void git_config_push_parameter(const char *text);
>  void git_config_push_env(const char *spec);
> +
> +/*
> + * Append `key=value` to the GIT_CONFIG_PARAMETERS value in `env`, quoted
> + * the way git_config_from_parameters() reads it, so that a child can be
> + * given configuration on top of what this process was given. A NULL
> + * `value` appends a boolean entry.
> + */
> +void git_config_append_parameter(struct strbuf *env, const char *key,
> +				 const char *value);

Pointing to that other function makes sense, but neither of the functions documents the actual format that's used.

Patrick