Skip to content

Make 'git config list --type=<X>' parse and filter types #2044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
derrickstolee wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Make 'git config list --type=<X>' parse and filter types #2044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bca83d8
config: move show_all_config()
derrickstolee Feb 10, 2026
93c94a1
config: add 'gently' parameter to format_config()
derrickstolee Feb 13, 2026
2b2a325
config: make 'git config list --type=<X>' work
derrickstolee Feb 10, 2026
4835ee5
config: format int64s gently
derrickstolee Feb 13, 2026
24eb757
config: format bools gently
derrickstolee Feb 13, 2026
02849ca
config: format bools or ints gently
derrickstolee Feb 13, 2026
9f06db2
config: format bools or strings in helper
derrickstolee Feb 13, 2026
d198c23
config: format paths gently
derrickstolee Feb 13, 2026
7568a0b
config: format expiry dates quietly
derrickstolee Feb 13, 2026
d98966f
color: add color_parse_quietly()
derrickstolee Feb 13, 2026
078065c
config: format colors quietly
derrickstolee Feb 13, 2026
76fc767
config: restructure format_config()
derrickstolee Feb 13, 2026
3c32c03
config: use an enum for type
derrickstolee Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Documentation/git-config.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ Valid `<type>`'s include:
that the given value is canonicalize-able as an ANSI color, but it is written
Copy link

@gitgitgadget gitgitgadget bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@gitgitgadget gitgitgadget bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@gitgitgadget gitgitgadget bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@gitgitgadget gitgitgadget bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

On Fri, Feb 13, 2026 at 11:55:08PM +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. This is our first use of the 'gently' parameter with a nonzero
> value.
> 
> When listing multiple values, our initial settings for the output format
> is different. Add a new init helper to specify the fact that keys should
> be shown and also add the default delimiters as they were unset in some
> cases.
> 
> If there is an error in parsing, then the row is not output.

It might make sense to document the rationale behind this decision in
the commit message.

> diff --git a/builtin/config.c b/builtin/config.c
> index b4c4228311..4c4c791883 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -318,21 +318,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, 1) >= 0)
> +		fwrite(formatted.buf, 1, formatted.len, stdout);

We could probably use puts(3p) instead, but as we know the length of the
data ahead of time it might be more efficient to use fwrite(3p) indeed.
Ultimately I guess it doesn't matter much.

Patrick

Copy link

@gitgitgadget gitgitgadget bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Patrick Steinhardt <ps@pks.im> writes:

>> -	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, 1) >= 0)
>> +		fwrite(formatted.buf, 1, formatted.len, stdout);
>
> We could probably use puts(3p) instead, but as we know the length of the
> data ahead of time it might be more efficient to use fwrite(3p) indeed.
> Ultimately I guess it doesn't matter much.
>
> Patrick

If we are not always doing LF-delimited output, puts(3) would not
help us very much, I suspect.

Copy link

@gitgitgadget gitgitgadget bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

On Tue, Feb 17, 2026 at 08:11:40AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> -	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, 1) >= 0)
> >> +		fwrite(formatted.buf, 1, formatted.len, stdout);
> >
> > We could probably use puts(3p) instead, but as we know the length of the
> > data ahead of time it might be more efficient to use fwrite(3p) indeed.
> > Ultimately I guess it doesn't matter much.
> >
> > Patrick
> 
> If we are not always doing LF-delimited output, puts(3) would not
> help us very much, I suspect.

D'oh, right.

Patrick

as-is.
+
If the command is in `list` mode, then the `--type <type>` argument will apply
to each listed config value. If the value does not successfully parse in that
format, then it will be omitted from the list.

--bool::
--int::
Expand Down
Loading
Loading