On Wed, Feb 11, 2026 at 12:49:19PM -0500, Derrick Stolee wrote:
Show 65 quoted lines
> 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.