[RFC] Make 'git config list --type=<X>' parse and filter types

[derrickstolee]

I started down this road based on feedback on my 'git config-batch' RFC [1].

[1] https://lore.kernel.org/git/pull.2033.git.1770214803.gitgitgadget@gmail.com/

I had described my intention to use 'git config-batch' as a single process to load multiple config values one-by-one. Brian mentioned that 'git config list -z' would probably suffice, so I started experimenting in that direction [2].

[2] git-ecosystem/git-credential-manager@main...derrickstolee:config-list

However, I ran into a problem: the most critical performance bottleneck is related to path-formatted config values that are queried with 'git config get --type=path -z'. It wasn't hard to update things to lazily load the full list of config values by type [3], but I then noticed a big problem!

[3] git-ecosystem/git-credential-manager@d403c8e

Problem: 'git config list' doesn't respect --type=<X>!

This boils down to the fact that the iterator function show_all_config() doesn't call format_config(), which includes the type-parsing code.

This wasn't super trivial to update:

1. format_config() uses git_config_parse_*() methods, which die() on a bad parse.
2. The path parsing code didn't have a gentle version.
3. The two paths ('git config list' and 'git config --list') needed to standardize their display options to work with format_config().
4. Finally, we need to filter out key-value pairs that don't match the given type.

This is marked as an RFC because I need to add some more tests and because this is a behavior change! If there are any tools currently passing the --type=<X> argument to git config list then they will have a change of behavior with this series. It's an easy workaround: drop the --type argument or add --no-type to go back to the previous behavior.

Thanks for any and all feedback,
-Stolee

cc: gitster@pobox.com
cc: "brian m. carlson" sandals@crustytoothpaste.net
cc: Phillip Wood phillip.wood123@gmail.com
cc: "Kristoffer Haugsbakk" kristofferhaugsbakk@fastmail.com
cc: Jean-Noël Avila jn.avila@free.fr
cc: Patrick Steinhardt ps@pks.im

[derrickstolee]

/submit

[gitgitgadget]

Submitted as pull.2044.git.1770698579.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2044/derrickstolee/config-list-type-v1

To fetch this version to local tag pr-2044/derrickstolee/config-list-type-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2044/derrickstolee/config-list-type-v1

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Problem: 'git config list' doesn't respect --type=<X>!

;-).  

As there is no "inherent" type associated with each configuration
variable (in other words, type of a particular configuration
variable is something determined by the caller that wants the value
of that variable), "git config list/get --type=auto" would not work,
but it would not be too bad to allow "git config list --type=path"
to treat everything as if it is a path and having to filter nonsense
out of the result (like "core.bare = true/false" or even "core.bare"
without value that means true, which may make the "*force*
interpreting it as path" approach to barf), which is an inevitable
consequence.

> This boils down to the fact that the iterator function show_all_config()
> doesn't call format_config(), which includes the type-parsing code.
>
> This wasn't super trivial to update:
>
>  1. format_config() uses git_config_parse_*() methods, which die() on a bad
>     parse.
>  2. The path parsing code didn't have a gentle version.
>  3. The two paths ('git config list' and 'git config --list') needed to
>     standardize their display options to work with format_config().

Thanks for dealing with them.  These are what I would have expected
as part of the "inevitable consequence".

>  4. Finally, we need to filter out key-value pairs that don't match the
>     given type.

This one, however, I need to see the actual code before commenting,
as I do not think key-value pairs have inherent types.  The _only_
special case where you can tell what type the thing is is the
valueless true, which we can safely say is inherently boolean.
Everything else is text string, sometimes interpreted as boolean,
sometimes number, sometimes human-scaled number, sometimes path
(with possible tilde expansion), etc.

> This is marked as an RFC because I need to add some more tests and because
> this is a behavior change! If there are any tools currently passing the
> --type=<X> argument to git config list then they will have a change of
> behavior with this series. It's an easy workaround: drop the --type argument
> or add --no-type to go back to the previous behavior.
>
> Thanks for any and all feedback, -Stolee
>
> Derrick Stolee (5):
>   config: move show_all_config()
>   parse: add git_parse_maybe_pathname()
>   config: allow format_config() to filter
>   config: create special init for list mode
>   config: make 'git config list --type=<X>' work
>
>  Documentation/git-config.adoc |   3 +
>  builtin/config.c              | 130 ++++++++++++++++++++++++----------
>  config.c                      |  14 +---
>  parse.c                       |  24 +++++++
>  parse.h                       |   2 +
>  t/t1300-config.sh             |  26 ++++++-
>  6 files changed, 147 insertions(+), 52 deletions(-)
>
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2044%2Fderrickstolee%2Fconfig-list-type-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2044/derrickstolee/config-list-type-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2044

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> The format_config() method in builtin/config.c currently only uses
> git_config_*() methods for parsing. This allows parsing errors to result
> in die() messages appropriate with keys in the error message.
>
> In a future change we will want to use format_config() within 'git
> config list' to help format the output, including when --type=<X>
> arguments are provided. When the parsing fails in that case, that
> key-value pair should be omitted instead of causing a failure across the
> entire command.
>
> This change is formatted in such a way that the if/else-if structure
> allows the default die_on_error version to appear first and then be
> followed by the gentle parsing mode immediately afterwards.
>
> The only callers right now have die_on_parse set to 1.

Certainly you meant die-on-parse-errors, not unconditionally die
when asked to parse ;-).

I wonder if a "bool gently" like everybody else takes would be
easier to understand by more developers and readers, though.



> +		if (opts->type == TYPE_INT && die_on_parse) {
>  			strbuf_addf(buf, "%"PRId64,
>  				    git_config_int64(key_, value_ ? value_ : "", kvi));
> +		} else if (opts->type == TYPE_INT) {
> +			int64_t v;
> +			int ret = git_parse_int64(value_, &v);
> +
> +			if (ret)
> +				return -1;
> +
> +			strbuf_addf(buf, "%"PRId64, v);
> +		}

So, this follows the typical layout that was described in the
proposed log message.  I wonder if it is too much to break the set
of helper functions further down so that this part of the caller can
say something like:

	switch (opts->type) {
	case TYPE_INT:
		format_config_int(buf, key_, value_, kvi, gently);
		break;

and similar case arms for other types?

[gitgitgadget]

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

On 2/10/2026 12:04 AM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The format_config() method in builtin/config.c currently only uses
>> git_config_*() methods for parsing. This allows parsing errors to result
>> in die() messages appropriate with keys in the error message.
>>
>> In a future change we will want to use format_config() within 'git
>> config list' to help format the output, including when --type=<X>
>> arguments are provided. When the parsing fails in that case, that
>> key-value pair should be omitted instead of causing a failure across the
>> entire command.
>>
>> This change is formatted in such a way that the if/else-if structure
>> allows the default die_on_error version to appear first and then be
>> followed by the gentle parsing mode immediately afterwards.
>>
>> The only callers right now have die_on_parse set to 1.
> 
> Certainly you meant die-on-parse-errors, not unconditionally die
> when asked to parse ;-).
> 
> I wonder if a "bool gently" like everybody else takes would be
> easier to understand by more developers and readers, though.

'gently' makes a lot more sense.

>> +		if (opts->type == TYPE_INT && die_on_parse) {
>>  			strbuf_addf(buf, "%"PRId64,
>>  				    git_config_int64(key_, value_ ? value_ : "", kvi));
>> +		} else if (opts->type == TYPE_INT) {
>> +			int64_t v;
>> +			int ret = git_parse_int64(value_, &v);
>> +
>> +			if (ret)
>> +				return -1;
>> +
>> +			strbuf_addf(buf, "%"PRId64, v);
>> +		}
> 
> So, this follows the typical layout that was described in the
> proposed log message.  I wonder if it is too much to break the set
> of helper functions further down so that this part of the caller can
> say something like:
> 
> 	switch (opts->type) {
> 	case TYPE_INT:
> 		format_config_int(buf, key_, value_, kvi, gently);
> 		break;
> 
> and similar case arms for other types?

I had a similar feeling that such a refactor would be necessary.

I didn't want to go through that careful work if it wasn't
justified by positive reactions to the RFC. Thanks for calling it
out, and I'll definitely put in that effort if we find this worth
a v2.

Thanks,
-Stolee

[gitgitgadget]

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

On 2/9/2026 11:59 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:...
>> This boils down to the fact that the iterator function show_all_config()
>> doesn't call format_config(), which includes the type-parsing code.
>>
>> This wasn't super trivial to update:
>>
>>  1. format_config() uses git_config_parse_*() methods, which die() on a bad
>>     parse.
>>  2. The path parsing code didn't have a gentle version.
>>  3. The two paths ('git config list' and 'git config --list') needed to
>>     standardize their display options to work with format_config().
> 
> Thanks for dealing with them.  These are what I would have expected
> as part of the "inevitable consequence".
> 
>>  4. Finally, we need to filter out key-value pairs that don't match the
>>     given type.
> 
> This one, however, I need to see the actual code before commenting,
> as I do not think key-value pairs have inherent types.  The _only_
> special case where you can tell what type the thing is is the
> valueless true, which we can safely say is inherently boolean.
> Everything else is text string, sometimes interpreted as boolean,
> sometimes number, sometimes human-scaled number, sometimes path
> (with possible tilde expansion), etc.
This is the crux of this series. If the caller asks for a type, then I
see a couple different ways to react:

 1. If the value fails to parse in that type, then don't list that
    result, allowing the caller to have confidence that every result
    is of the correct format.

 2. If the value fails to parse in that type, then list it in its
    base string. The caller would need to do extra parsing to check
    that the results match the correct format.

I chose option 1. It avoids showing results that would result in
'git config get --type=<X> <key>' to die().

I'd be interested to hear if there are reasons to go with option 2,
or if there exists an alternative option that I don't see.

Reordering your message somewhat:

> As there is no "inherent" type associated with each configuration
> variable (in other words, type of a particular configuration
> variable is something determined by the caller that wants the value
> of that variable), "git config list/get --type=auto" would not work,
> but it would not be too bad to allow "git config list --type=path"
> to treat everything as if it is a path and having to filter nonsense
> out of the result (like "core.bare = true/false" or even "core.bare"
> without value that means true, which may make the "*force*
> interpreting it as path" approach to barf), which is an inevitable
> consequence.

I agree that there is not inherent type, so the user can only
specify the type that they are expecting. To me, this is a request to
filter the results.

I don't think we'll have much success if we try to guess the type,
such as trying to parse an int, then a bool, then a path.

Thanks,
-Stolee

[gitgitgadget]

This patch series was integrated into seen via git@c6d7b35.

[gitgitgadget]

This patch series was integrated into seen via git@923ffa2.

[gitgitgadget]

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Tue, Feb 10, 2026 at 04:42:54AM +0000, Derrick Stolee via GitGitGadget wrote:
> This is marked as an RFC because I need to add some more tests and because
> this is a behavior change! If there are any tools currently passing the
> --type=<X> argument to git config list then they will have a change of
> behavior with this series. It's an easy workaround: drop the --type argument
> or add --no-type to go back to the previous behavior.

I think this is not a huge problem. It simply reads like a bug to me
that the command accepts the option, but doesn't honor it. Sure, it can
lead to different behaviour, but I think that's acceptable.

Patrick

[gitgitgadget]

User Patrick Steinhardt <ps@pks.im> has been added to the cc: list.

[gitgitgadget]

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Tue, Feb 10, 2026 at 04:42:56AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> This extraction of logic from config.c's git_config_pathname() allows
> for parsing a fully-qualified path from a relative path along with
> validation of the existence of the path without failing with a die().

That sentence is quite something. I had to read it thrice to understand
what it wants to say :)

> diff --git a/parse.c b/parse.c
> index 48313571aa..3f37f0b93a 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
>  		die(_("failed to parse %s"), k);
>  	return val;
>  }
> +
> +int git_parse_maybe_pathname(const char *value, char **dest)
> +{
> +	bool is_optional;
> +	char *path;
> +
> +	if (!value)
> +		return -1;
> +
> +	is_optional = skip_prefix(value, ":(optional)", &value);
> +	path = interpolate_path(value, 0);
> +	if (!path)
> +		return -1;
> +
> +	if (is_optional && is_missing_file(path)) {
> +		free(path);
> +		*dest = NULL;
> +		return 0;
> +	}
> +
> +	*dest = path;
> +	return 0;
> +}

Okay. So the difference is that this function here doesn't cause us to
die in case the path is not marked as optional and missing. Makes sense.

> diff --git a/parse.h b/parse.h
> index ea32de9a91..4f97c3727a 100644
> --- a/parse.h
> +++ b/parse.h
> @@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value);
>  int git_env_bool(const char *, int);
>  unsigned long git_env_ulong(const char *, unsigned long);
>  
> +int git_parse_maybe_pathname(const char *value, char **dest);

I think this function could use some explanation what it actually does,
as the behaviour is non-trivial:

  - I think the ":(optional)" part needs to be documented properly to
    say that we return successfully with a NULL string in case the
    target path doesn't exist.

  - We should document that it expands "~" and "%(prefix)" (even though
    the latter feels somewhat coincidental to me).

  - The path is not resolved to an absolute path.

Thanks!

Patrick

[gitgitgadget]

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> Previously, the --type=<X> argument to 'git config list' was ignored and
> did nothing. Now, we add the use of format_config() to the
> show_all_config() function so each key-value pair is attempted to be
> parsed.
> 
> If there is an error in parsing, then the row is not output.

I was a bit surprised at first, but now that I think about it a bit more
I think this is sensible behaviour. If I ask for `git config list
--type=int`, then I don't want to see any non-int configuration. I
wouldn't even expect a warning, as the option essentially works like a
filter.

> This is a change in behavior! We are starting to respect an option that
> was previously ignored, leading to potential user confusion. This is
> probably still a good option, since the --type argument did not change
> behavior at all previously, so users can get the behavior they expect by
> removing the --type argument or adding the --no-type argument.

Yeah, I fully agree that this is a sensible change in behaviour. It is
obviously broken right now, so I would claim that this is simply a bug
fix.

> diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
> index ac3b536a15..5300dd4c51 100644
> --- a/Documentation/git-config.adoc
> +++ b/Documentation/git-config.adoc

The synopsis of `git config list` should also be amended.

> diff --git a/builtin/config.c b/builtin/config.c
> index e69b26af6a..c83514b4ff 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
>  {
>  	const struct config_display_options *opts = cb;
>  	const struct key_value_info *kvi = ctx->kvi;
> +	struct strbuf formatted = STRBUF_INIT;
>  
> -	if (opts->show_origin || opts->show_scope) {
> -		struct strbuf buf = STRBUF_INIT;
> -		if (opts->show_scope)
> -			show_config_scope(opts, kvi, &buf);
> -		if (opts->show_origin)
> -			show_config_origin(opts, kvi, &buf);
> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> -		fwrite(buf.buf, 1, buf.len, stdout);
> -		strbuf_release(&buf);
> -	}
> -	if (!opts->omit_values && value_)
> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> -	else
> -		printf("%s%c", key_, opts->term);
> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> +
> +	strbuf_release(&formatted);
>  	return 0;
>  }
>  

I wonder whether there is a good argument to be made here that we should
keep the old logic in case no "--type=" parameter was given. In that
case, for example the following output would remain the same:

> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 9850fcd5b5..b5ce900126 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -2459,9 +2459,10 @@ done
>  
>  cat >.git/config <<-\EOF &&
>  [section]
> -foo = true
> +foo = True
>  number = 10
>  big = 1M
> +path = ~/dir
>  EOF
>  
>  test_expect_success 'identical modern --type specifiers are allowed' '

I'm not really sure whether we want that though. I actually like that
this also leads to some code duplication, so maybe this is fine?

Patrick

[gitgitgadget]

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

On 2/11/2026 7:13 AM, Patrick Steinhardt wrote:
> On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:

>> diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
>> index ac3b536a15..5300dd4c51 100644
>> --- a/Documentation/git-config.adoc
>> +++ b/Documentation/git-config.adoc
> 
> The synopsis of `git config list` should also be amended.

Good point. Will fix.
 
>> diff --git a/builtin/config.c b/builtin/config.c
>> index e69b26af6a..c83514b4ff 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
>>  {
>>  	const struct config_display_options *opts = cb;
>>  	const struct key_value_info *kvi = ctx->kvi;
>> +	struct strbuf formatted = STRBUF_INIT;
>>  
>> -	if (opts->show_origin || opts->show_scope) {
>> -		struct strbuf buf = STRBUF_INIT;
>> -		if (opts->show_scope)
>> -			show_config_scope(opts, kvi, &buf);
>> -		if (opts->show_origin)
>> -			show_config_origin(opts, kvi, &buf);
>> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
>> -		fwrite(buf.buf, 1, buf.len, stdout);
>> -		strbuf_release(&buf);
>> -	}
>> -	if (!opts->omit_values && value_)
>> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
>> -	else
>> -		printf("%s%c", key_, opts->term);
>> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
>> +		fwrite(formatted.buf, 1, formatted.len, stdout);
>> +
>> +	strbuf_release(&formatted);
>>  	return 0;
>>  }
>>  
> 
> I wonder whether there is a good argument to be made here that we should
> keep the old logic in case no "--type=" parameter was given. In that
> case, for example the following output would remain the same:

If no `--type=` parameter is given, then this new implementation does
the exact same thing as the display_options use a string format (which
does not mutate the config values).

>> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
>> index 9850fcd5b5..b5ce900126 100755
>> --- a/t/t1300-config.sh
>> +++ b/t/t1300-config.sh
>> @@ -2459,9 +2459,10 @@ done
>>  
>>  cat >.git/config <<-\EOF &&
>>  [section]
>> -foo = true
>> +foo = True
>>  number = 10
>>  big = 1M
>> +path = ~/dir
>>  EOF
>>  
>>  test_expect_success 'identical modern --type specifiers are allowed' '
> 
> I'm not really sure whether we want that though. I actually like that
> this also leads to some code duplication, so maybe this is fine?

The change you highlight here is a difference in the config file _contents_
and not the expected output. These changes are to help demonstrate that the
bool and path types make meaningful conversions when listing these values.

The previous tests for getting bool values did not demonstrate the way it
modifies case, for example.

Thanks,
-Stolee

[gitgitgadget]

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Wed, Feb 11, 2026 at 12:49:19PM -0500, Derrick Stolee wrote:
> On 2/11/2026 7:13 AM, Patrick Steinhardt wrote:
> > On Tue, Feb 10, 2026 at 04:42:59AM +0000, Derrick Stolee via GitGitGadget wrote:
> >> diff --git a/builtin/config.c b/builtin/config.c
> >> index e69b26af6a..c83514b4ff 100644
> >> --- a/builtin/config.c
> >> +++ b/builtin/config.c
> >> @@ -363,21 +363,12 @@ static int show_all_config(const char *key_, const char *value_,
> >>  {
> >>  	const struct config_display_options *opts = cb;
> >>  	const struct key_value_info *kvi = ctx->kvi;
> >> +	struct strbuf formatted = STRBUF_INIT;
> >>  
> >> -	if (opts->show_origin || opts->show_scope) {
> >> -		struct strbuf buf = STRBUF_INIT;
> >> -		if (opts->show_scope)
> >> -			show_config_scope(opts, kvi, &buf);
> >> -		if (opts->show_origin)
> >> -			show_config_origin(opts, kvi, &buf);
> >> -		/* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> >> -		fwrite(buf.buf, 1, buf.len, stdout);
> >> -		strbuf_release(&buf);
> >> -	}
> >> -	if (!opts->omit_values && value_)
> >> -		printf("%s%c%s%c", key_, opts->delim, value_, opts->term);
> >> -	else
> >> -		printf("%s%c", key_, opts->term);
> >> +	if (format_config(opts, &formatted, key_, value_, kvi, 0) >= 0)
> >> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> >> +
> >> +	strbuf_release(&formatted);
> >>  	return 0;
> >>  }
> >>  
> > 
> > I wonder whether there is a good argument to be made here that we should
> > keep the old logic in case no "--type=" parameter was given. In that
> > case, for example the following output would remain the same:
> 
> If no `--type=` parameter is given, then this new implementation does
> the exact same thing as the display_options use a string format (which
> does not mutate the config values).
> 
> >> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> >> index 9850fcd5b5..b5ce900126 100755
> >> --- a/t/t1300-config.sh
> >> +++ b/t/t1300-config.sh
> >> @@ -2459,9 +2459,10 @@ done
> >>  
> >>  cat >.git/config <<-\EOF &&
> >>  [section]
> >> -foo = true
> >> +foo = True
> >>  number = 10
> >>  big = 1M
> >> +path = ~/dir
> >>  EOF
> >>  
> >>  test_expect_success 'identical modern --type specifiers are allowed' '
> > 
> > I'm not really sure whether we want that though. I actually like that
> > this also leads to some code duplication, so maybe this is fine?
> 
> The change you highlight here is a difference in the config file _contents_
> and not the expected output. These changes are to help demonstrate that the
> bool and path types make meaningful conversions when listing these values.

Ooh, right. Completely missed that, thanks for the clarification.

Patrick