From 77b479c0ca1b97cf1b88b71d3f18f45a5d0a8a95 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 11 Sep 2025 12:48:26 +0200 Subject: [PATCH 1/6] Changes in v4: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v3: https://lore.kernel.org/r/20250918-pks-config-color-v3-0-08ea618cae26@pks.im MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit builtin/config: bug fixes for "get" subcommand with "--type=color" Hi, this small patch series contains two bug fixes for `git config get --type=color`: - We restore the behaviour where we can now parse colors without a config key. - We stop spawning the pager when the user requests to print ANSI color escape sequences. Furthermore, the patch series does some lighter refactorings of t1300. That test file still has its fair share of issues, but at least it looks a bit less dirty now. Changes in v2: - Improve commit messages. - Use "\EOF" and "-EOF" in more cases. - Move a style fixup from the first commit into the second commit. - Link to v1: https://lore.kernel.org/r/20250911-pks-config-color-v1-0-3a7c79df65b1@pks.im Changes in v3: - Provide additional context as part of the commit message for the commit that stops setting up the pager with `--type=color`. - Link to v2: https://lore.kernel.org/r/20250915-pks-config-color-v2-0-e4290bd8d13c@pks.im Thanks! Patrick To: git@vger.kernel.org Cc: SZEDER Gábor Cc: Junio C Hamano Cc: Kristoffer Haugsbakk --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 4, "change-id": "20250911-pks-config-color-e5b8a213e895", "prefixes": [], "history": { "v1": [ "20250911-pks-config-color-v1-0-3a7c79df65b1@pks.im" ], "v2": [ "20250915-pks-config-color-v2-0-e4290bd8d13c@pks.im" ], "v3": [ "20250918-pks-config-color-v3-0-08ea618cae26@pks.im" ] } } } -- GitLab From 77f49eeeb9457cc840dc3756aadd33bd5efbe53e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 11 Sep 2025 12:19:34 +0200 Subject: [PATCH 2/6] t1300: write test expectations in the test's body There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies. Convert those to instead do so as part of the test itself. While at it, normalize these tests to use `<<\EOF` for those that don't use variable expansion and `<<-EOF` for those that aren't sensitive to indentation. Note that there are two exceptions that we leave as-is for now since they are reused across tests. Signed-off-by: Patrick Steinhardt --- t/t1300-config.sh | 291 ++++++++++++++++++++-------------------------- 1 file changed, 128 insertions(+), 163 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index f8568218392..538f2c9b8a0 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -134,38 +134,39 @@ test_expect_success 'clear default config' ' rm -f .git/config ' -cat > expect << EOF +test_expect_success 'initial' ' + cat >expect <<\EOF && [section] penguin = little blue EOF -test_expect_success 'initial' ' git config ${mode_set} section.penguin "little blue" && test_cmp expect .git/config ' -cat > expect << EOF +test_expect_success 'mixed case' ' + cat >expect <<\EOF && [section] penguin = little blue Movie = BadPhysics EOF -test_expect_success 'mixed case' ' git config ${mode_set} Section.Movie BadPhysics && test_cmp expect .git/config ' -cat > expect << EOF +test_expect_success 'similar section' ' + cat >expect <<\EOF && [section] penguin = little blue Movie = BadPhysics [Sections] WhatEver = Second EOF -test_expect_success 'similar section' ' git config ${mode_set} Sections.WhatEver Second && test_cmp expect .git/config ' -cat > expect << EOF +test_expect_success 'uppercase section' ' + cat >expect <<\EOF && [section] penguin = little blue Movie = BadPhysics @@ -173,7 +174,6 @@ cat > expect << EOF [Sections] WhatEver = Second EOF -test_expect_success 'uppercase section' ' git config ${mode_set} SECTION.UPPERCASE true && test_cmp expect .git/config ' @@ -186,7 +186,8 @@ test_expect_success 'replace with non-match (actually matching)' ' git config section.penguin "very blue" !kingpin ' -cat > expect << EOF +test_expect_success 'append comments' ' + cat >expect <<\EOF && [section] Movie = BadPhysics UPPERCASE = true @@ -198,8 +199,6 @@ cat > expect << EOF [Sections] WhatEver = Second EOF - -test_expect_success 'append comments' ' git config --replace-all --comment="Pygoscelis papua" section.penguin gentoo && git config ${mode_set} --comment="find fish" section.disposition peckish && git config ${mode_set} --comment="#abc" section.foo bar && @@ -265,14 +264,15 @@ test_expect_success 'unset with cont. lines' ' git config ${mode_unset} beta.baz ' -cat > expect <<\EOF -[alpha] -bar = foo -[beta] -foo = bar -EOF - -test_expect_success 'unset with cont. lines is correct' 'test_cmp expect .git/config' +test_expect_success 'unset with cont. lines is correct' ' + cat >expect <<-\EOF && + [alpha] + bar = foo + [beta] + foo = bar + EOF + test_cmp expect .git/config +' cat > .git/config << EOF [beta] ; silly comment # another comment @@ -292,16 +292,15 @@ test_expect_success 'multiple unset' ' git config ${mode_unset_all} beta.haha ' -cat > expect << EOF +test_expect_success 'multiple unset is correct' ' + cat >expect < expect << EOF +test_expect_success 'all replaced' ' + cat >expect < expect << EOF +test_expect_success 'really mean test' ' + cat >expect < expect << EOF +test_expect_success 'really really mean test' ' + cat >expect < expect << EOF +test_expect_success 'unset' ' + cat >expect < expect << EOF +test_expect_success 'multivar' ' + cat >expect < expect << EOF +test_expect_success 'multivar replace' ' + cat >expect < expect << EOF +test_expect_success 'multivar unset' ' + cat >expect < expect << EOF +test_expect_success 'hierarchical section value' ' + cat >expect < expect << EOF -beta.noindent=sillyValue -nextsection.nonewline=wow2 for me -123456.a123=987 -version.1.2.3eX.alpha=beta -EOF - test_expect_success 'working --list' ' + cat >expect <<-\EOF && + beta.noindent=sillyValue + nextsection.nonewline=wow2 for me + 123456.a123=987 + version.1.2.3eX.alpha=beta + EOF git config ${mode_prefix}list > output && test_cmp expect output ' @@ -500,44 +495,40 @@ test_expect_success '--list without repo produces empty output' ' test_must_be_empty output ' -cat > expect << EOF -beta.noindent -nextsection.nonewline -123456.a123 -version.1.2.3eX.alpha -EOF - test_expect_success '--name-only --list' ' + cat >expect <<-\EOF && + beta.noindent + nextsection.nonewline + 123456.a123 + version.1.2.3eX.alpha + EOF git config ${mode_prefix}list --name-only >output && test_cmp expect output ' -cat > expect << EOF -beta.noindent sillyValue -nextsection.nonewline wow2 for me -EOF - test_expect_success '--get-regexp' ' + cat >expect <<-\EOF && + beta.noindent sillyValue + nextsection.nonewline wow2 for me + EOF git config ${mode_get_regexp} in >output && test_cmp expect output ' -cat > expect << EOF -beta.noindent -nextsection.nonewline -EOF - test_expect_success '--name-only --get-regexp' ' + cat >expect <<-\EOF && + beta.noindent + nextsection.nonewline + EOF git config ${mode_get_regexp} --name-only in >output && test_cmp expect output ' -cat > expect << EOF -wow2 for me -wow4 for you -EOF - test_expect_success '--add' ' + cat >expect <<-\EOF && + wow2 for me + wow4 for you + EOF git config --add nextsection.nonewline "wow4 for you" && git config ${mode_get_all} nextsection.nonewline > output && test_cmp expect output @@ -558,37 +549,32 @@ test_expect_success 'get variable with empty value' ' git config --get emptyvalue.variable ^$ ' -echo novalue.variable > expect - test_expect_success 'get-regexp variable with no value' ' + echo novalue.variable >expect && git config ${mode_get_regexp} novalue > output && test_cmp expect output ' -echo 'novalue.variable true' > expect - test_expect_success 'get-regexp --bool variable with no value' ' + echo "novalue.variable true" >expect && git config ${mode_get_regexp} --bool novalue > output && test_cmp expect output ' -echo 'emptyvalue.variable ' > expect - test_expect_success 'get-regexp variable with empty value' ' + echo "emptyvalue.variable " >expect && git config ${mode_get_regexp} emptyvalue > output && test_cmp expect output ' -echo true > expect - test_expect_success 'get bool variable with no value' ' + echo true >expect && git config --bool novalue.variable > output && test_cmp expect output ' -echo false > expect - test_expect_success 'get bool variable with empty value' ' + echo false >expect && git config --bool emptyvalue.variable > output && test_cmp expect output ' @@ -604,19 +590,19 @@ cat > .git/config << EOF c = d EOF -cat > expect << EOF +test_expect_success 'new section is partial match of another' ' + cat >expect <<\EOF && [a.b] c = d [a] x = y EOF - -test_expect_success 'new section is partial match of another' ' git config a.x y && test_cmp expect .git/config ' -cat > expect << EOF +test_expect_success 'new variable inserts into proper section' ' + cat >expect <<\EOF && [a.b] c = d [a] @@ -625,8 +611,6 @@ cat > expect << EOF [b] x = y EOF - -test_expect_success 'new variable inserts into proper section' ' git config b.x y && git config a.b c && test_cmp expect .git/config @@ -642,11 +626,10 @@ cat > other-config << EOF bahn = strasse EOF -cat > expect << EOF -ein.bahn=strasse -EOF - test_expect_success 'alternative GIT_CONFIG' ' + cat >expect <<-\EOF && + ein.bahn=strasse + EOF GIT_CONFIG=other-config git config ${mode_prefix}list >output && test_cmp expect output ' @@ -675,14 +658,13 @@ test_expect_success 'refer config from subdirectory' ' test_cmp_config -C x strasse --file=../other-config --get ein.bahn ' -cat > expect << EOF +test_expect_success '--set in alternative file' ' + cat >expect <<\EOF && [ein] bahn = strasse [anwohner] park = ausweis EOF - -test_expect_success '--set in alternative file' ' git config --file=other-config anwohner.park ausweis && test_cmp expect other-config ' @@ -730,7 +712,8 @@ test_expect_success 'rename another section' ' git config ${mode_prefix}rename-section branch."1 234 blabl/a" branch.drei ' -cat > expect << EOF +test_expect_success 'rename succeeded' ' + cat >expect <<\EOF && # Hallo #Bello [branch "zwei"] @@ -740,8 +723,6 @@ cat > expect << EOF [branch "drei"] weird EOF - -test_expect_success 'rename succeeded' ' test_cmp expect .git/config ' @@ -753,7 +734,8 @@ test_expect_success 'rename a section with a var on the same line' ' git config ${mode_prefix}rename-section branch.vier branch.zwei ' -cat > expect << EOF +test_expect_success 'rename succeeded' ' + cat >expect <<\EOF && # Hallo #Bello [branch "zwei"] @@ -765,8 +747,6 @@ weird [branch "zwei"] z = 1 EOF - -test_expect_success 'rename succeeded' ' test_cmp expect .git/config ' @@ -816,32 +796,29 @@ test_expect_success 'remove section' ' git config ${mode_prefix}remove-section branch.zwei ' -cat > expect << EOF +test_expect_success 'section was removed properly' ' + cat >expect <<\EOF && # Hallo #Bello [branch "drei"] weird EOF - -test_expect_success 'section was removed properly' ' test_cmp expect .git/config ' -cat > expect << EOF +test_expect_success 'section ending' ' + cat >expect <<\EOF && [gitcvs] enabled = true dbname = %Ggitcvs2.%a.%m.sqlite [gitcvs "ext"] dbname = %Ggitcvs1.%a.%m.sqlite EOF - -test_expect_success 'section ending' ' rm -f .git/config && git config ${mode_set} gitcvs.enabled true && git config ${mode_set} gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite && git config ${mode_set} gitcvs.dbname %Ggitcvs2.%a.%m.sqlite && test_cmp expect .git/config - ' test_expect_success numbers ' @@ -885,19 +862,17 @@ test_expect_success 'invalid stdin config' ' test_grep "bad config line 1 in standard input" output ' -cat > expect << EOF -true -false -true -false -true -false -true -false -EOF - test_expect_success bool ' - + cat >expect <<-\EOF && + true + false + true + false + true + false + true + false + EOF git config ${mode_set} bool.true1 01 && git config ${mode_set} bool.true2 -1 && git config ${mode_set} bool.true3 YeS && @@ -923,7 +898,8 @@ test_expect_success 'invalid bool (set)' ' test_must_fail git config --bool bool.nobool foobar' -cat > expect <<\EOF +test_expect_success 'set --bool' ' + cat >expect <<\EOF && [bool] true1 = true true2 = true @@ -934,9 +910,6 @@ cat > expect <<\EOF false3 = false false4 = false EOF - -test_expect_success 'set --bool' ' - rm -f .git/config && git config --bool bool.true1 01 && git config --bool bool.true2 -1 && @@ -948,15 +921,13 @@ test_expect_success 'set --bool' ' git config --bool bool.false4 FALSE && test_cmp expect .git/config' -cat > expect <<\EOF +test_expect_success 'set --int' ' + cat >expect <<\EOF && [int] val1 = 1 val2 = -1 val3 = 5242880 EOF - -test_expect_success 'set --int' ' - rm -f .git/config && git config --int int.val1 01 && git config --int int.val2 -1 && @@ -994,7 +965,8 @@ test_expect_success 'get --bool-or-int' ' test_cmp expect actual ' -cat >expect <<\EOF +test_expect_success 'set --bool-or-int' ' + cat >expect <<\EOF && [bool] true1 = true false1 = false @@ -1005,8 +977,6 @@ cat >expect <<\EOF int2 = 1 int3 = -1 EOF - -test_expect_success 'set --bool-or-int' ' rm -f .git/config && git config --bool-or-int bool.true1 true && git config --bool-or-int bool.false1 false && @@ -1018,14 +988,13 @@ test_expect_success 'set --bool-or-int' ' test_cmp expect .git/config ' -cat >expect <<\EOF +test_expect_success !MINGW 'set --path' ' + cat >expect <<\EOF && [path] home = ~/ normal = /dev/null trailingtilde = foo~ EOF - -test_expect_success !MINGW 'set --path' ' rm -f .git/config && git config --path path.home "~/" && git config --path path.normal "/dev/null" && @@ -1037,25 +1006,23 @@ then test_set_prereq HOMEVAR fi -cat >expect <expect <<-EOF && + $HOME/ + /dev/null + foo~ + EOF git config --get --path path.home > result && git config --get --path path.normal >> result && git config --get --path path.trailingtilde >> result && test_cmp expect result ' -cat >expect <<\EOF -/dev/null -foo~ -EOF - test_expect_success !MINGW 'get --path copes with unset $HOME' ' + cat >expect <<-\EOF && + /dev/null + foo~ + EOF ( sane_unset HOME && test_must_fail git config --get --path path.home \ @@ -1112,12 +1079,11 @@ test_expect_success 'get --type=color' ' test_cmp expect actual ' -cat >expect << EOF +test_expect_success 'set --type=color' ' + cat >expect <<\EOF && [foo] color = red EOF - -test_expect_success 'set --type=color' ' rm .git/config && git config --type=color foo.color "red" && test_cmp expect .git/config @@ -1133,14 +1099,14 @@ test_expect_success 'set --type=color barfs on non-color' ' test_grep "cannot parse color" error ' -cat > expect << EOF +test_expect_success 'quoting' ' + cat >expect <<\EOF && [quote] leading = " test" ending = "test " semicolon = "test;test" hash = "test#test" EOF -test_expect_success 'quoting' ' rm -f .git/config && git config ${mode_set} quote.leading " test" && git config ${mode_set} quote.ending "test " && @@ -1166,13 +1132,12 @@ inued inued" EOF -cat > expect <<\EOF -section.continued=continued -section.noncont=not continued -section.quotecont=cont;inued -EOF - test_expect_success 'value continued on next line' ' + cat >expect <<-\EOF && + section.continued=continued + section.noncont=not continued + section.quotecont=cont;inued + EOF git config ${mode_prefix}list > result && test_cmp expect result ' -- GitLab From 57d61adeb7982b5f0eda1c7ae458ae13c1a32ee7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Sep 2025 13:28:22 +0200 Subject: [PATCH 3/6] t1300: small style fixups We have a couple of small style violations in t1300: - An empty newline at the start of the test body. - The test command is sometimes on the same line as the test name. - The closing single-quote is sometimes on the same line as the last command of the test. Fix these. Signed-off-by: Patrick Steinhardt --- t/t1300-config.sh | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 538f2c9b8a0..6d1015acfd8 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -213,7 +213,9 @@ test_expect_success 'Prohibited LF in comment' ' test_must_fail git config ${mode_set} --comment="a${LF}b" section.k v ' -test_expect_success 'non-match result' 'test_cmp expect .git/config' +test_expect_success 'non-match result' ' + test_cmp expect .git/config +' test_expect_success 'find mixed-case key by canonical name' ' test_cmp_config Second sections.whatever @@ -455,9 +457,13 @@ EOF test_cmp expect .git/config ' -test_expect_success 'invalid key' 'test_must_fail git config inval.2key blabla' +test_expect_success 'invalid key' ' + test_must_fail git config inval.2key blabla +' -test_expect_success 'correct key' 'git config 123456.a123 987' +test_expect_success 'correct key' ' + git config 123456.a123 987 +' test_expect_success 'hierarchical section' ' git config Version.1.2.3eX.Alpha beta @@ -490,6 +496,7 @@ test_expect_success 'working --list' ' git config ${mode_prefix}list > output && test_cmp expect output ' + test_expect_success '--list without repo produces empty output' ' git --git-dir=nonexistent config ${mode_prefix}list >output && test_must_be_empty output @@ -887,16 +894,17 @@ test_expect_success bool ' git config --bool --get bool.true$i >>result && git config --bool --get bool.false$i >>result || return 1 done && - test_cmp expect result' + test_cmp expect result +' test_expect_success 'invalid bool (--get)' ' - git config ${mode_set} bool.nobool foobar && - test_must_fail git config --bool --get bool.nobool' + test_must_fail git config --bool --get bool.nobool +' test_expect_success 'invalid bool (set)' ' - - test_must_fail git config --bool bool.nobool foobar' + test_must_fail git config --bool bool.nobool foobar +' test_expect_success 'set --bool' ' cat >expect <<\EOF && @@ -999,7 +1007,8 @@ EOF git config --path path.home "~/" && git config --path path.normal "/dev/null" && git config --path path.trailingtilde "foo~" && - test_cmp expect .git/config' + test_cmp expect .git/config +' if test_have_prereq !MINGW && test "${HOME+set}" then @@ -1117,10 +1126,13 @@ EOF test_expect_success 'key with newline' ' test_must_fail git config ${mode_get} "key.with -newline" 123' +newline" 123 +' -test_expect_success 'value with newline' 'git config ${mode_set} key.sub value.with\\\ -newline' +test_expect_success 'value with newline' ' + git config ${mode_set} key.sub value.with\\\ +newline +' cat > .git/config <<\EOF [section] @@ -1330,7 +1342,6 @@ test_expect_success 'multiple git -c appends config' ' ' test_expect_success 'last one wins: two level vars' ' - # sec.var and sec.VAR are the same variable, as the first # and the last level of a configuration variable name is # case insensitive. @@ -1349,7 +1360,6 @@ test_expect_success 'last one wins: two level vars' ' ' test_expect_success 'last one wins: three level vars' ' - # v.a.r and v.A.r are not the same variable, as the middle # level of a three-level configuration variable name is # case sensitive. -- GitLab From 2862b250157257b3f7837be3ad667748edb203a0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 11 Sep 2025 11:32:15 +0200 Subject: [PATCH 4/6] builtin/config: do not die in `get_color()` When trying to parse an invalid color via `get_color()` we die. We're about to introduce another caller in a subsequent commit though that has its own error handling, so dying is a bit drastic there. Furthermore, the only caller that we already have right now already knows to handle errors in other branches that don't call `get_color()`. Convert the function to instead return an error code to improve its flexibility. Signed-off-by: Patrick Steinhardt --- builtin/config.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 59fb113b073..afd48bfa51b 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -547,24 +547,31 @@ static int git_get_color_config(const char *var, const char *value, return 0; } -static void get_color(const struct config_location_options *opts, +static int get_color(const struct config_location_options *opts, const char *var, const char *def_color) { struct get_color_config_data data = { .get_color_slot = var, .parsed_color[0] = '\0', }; + int ret; config_with_options(git_get_color_config, &data, &opts->source, the_repository, &opts->options); if (!data.get_color_found && def_color) { - if (color_parse(def_color, data.parsed_color) < 0) - die(_("unable to parse default color value")); + if (color_parse(def_color, data.parsed_color) < 0) { + ret = error(_("unable to parse default color value")); + goto out; + } } + ret = 0; + +out: fputs(data.parsed_color, stdout); + return ret; } struct get_colorbool_config_data { @@ -1390,7 +1397,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix) } else if (actions == ACTION_GET_COLOR) { check_argc(argc, 1, 2); - get_color(&location_opts, argv[0], argv[1]); + ret = get_color(&location_opts, argv[0], argv[1]); } else if (actions == ACTION_GET_COLORBOOL) { check_argc(argc, 1, 2); -- GitLab From 0c79bec3e258cae688115d973ac8066d3d640939 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 11 Sep 2025 12:37:42 +0200 Subject: [PATCH 5/6] builtin/config: special-case retrieving colors without a key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our documentation for git-config(1) has a section where it explains how to parse and use colors as Git would configure them. In order to get the ANSI color escape sequence to reset the colors to normal we recommend the following command: $ git config get --type=color --default="reset" "" This command is not supposed to parse any configuration keys. Instead, it is expected to parse the "reset" default value and turn it into a proper ANSI color escape sequence. It was reported though [1] that this command doesn't work: $ git config get --type=color --default="reset" "" error: key does not contain a section: This error was introduced in 4e51389000 (builtin/config: introduce "get" subcommand, 2024-05-06), where we introduced the "get" subcommand to retrieve configuration values. The preimage of that commit used `git config --get-color "" "reset"` instead, which still works. This use case is really quite specific to parsing colors, as it wouldn't make sense to give git-config(1) a default value and an empty config key only to return that default value unmodified. But with `--type=color` we don't return the value directly; we instead parse the value into an ANSI escape sequence. As such, we can easily special-case this one use case: - If the provided config key is empty; - the user is asking for a color code and the user; and - the user has provided a default value, then we call `get_color()` directly. Do so to make the documented command work as expected. [1]: Reported-by: SZEDER Gábor Signed-off-by: Patrick Steinhardt --- builtin/config.c | 2 ++ t/t1300-config.sh | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/builtin/config.c b/builtin/config.c index afd48bfa51b..f50c11df575 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -923,6 +923,8 @@ static int cmd_config_get(int argc, const char **argv, const char *prefix, if (url) ret = get_urlmatch(&location_opts, &display_opts, argv[0], url); + else if (display_opts.type == TYPE_COLOR && !strlen(argv[0]) && display_opts.default_value) + ret = get_color(&location_opts, "", display_opts.default_value); else ret = get_value(&location_opts, &display_opts, argv[0], value_pattern, get_value_flags, flags); diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 6d1015acfd8..3cf5d17abab 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1083,11 +1083,22 @@ test_expect_success 'get --type=color' ' rm .git/config && git config ${mode_set} foo.color "red" && git config --get --type=color foo.color >actual.raw && + git config get --type=color foo.color >actual-subcommand.raw && + test_cmp actual.raw actual-subcommand.raw && test_decode_color actual && echo "" >expect && test_cmp expect actual ' +test_expect_success 'get --type=color with default value only' ' + git config --get-color "" "red" >actual.raw && + test_decode_color actual && + echo "" >expect && + test_cmp expect actual && + git config get --type=color --default="red" "" >actual-subcommand.raw && + test_cmp actual.raw actual-subcommand.raw +' + test_expect_success 'set --type=color' ' cat >expect <<\EOF && [foo] -- GitLab From b6308f9df60b7b4e4f215135cf9b24fead5adf26 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 11 Sep 2025 12:48:18 +0200 Subject: [PATCH 6/6] builtin/config: do not spawn pager when printing color codes With `git config get --type=color` the user asks us to parse a specific configuration key and turn the value into an ANSI color escape sequence. The printed string can then for example be used as part of shell scripts to reuse the same colors as Git. Right now though we set up the auto-pager, which means that the string may be written to the pager instead of directly to the terminal. This behaviour is problematic for two reasons: - Color codes are meant for direct terminal output; writing them into a pager does not seem like a sensible thing to do without additional text. - It is inconsistent with `git config --get-color`, which never uses a pager, despite the fact that we claim `git config get --type=color` to be a drop-in replacement in git-config(1). Fix this by disabling the pager when outputting color sequences. Signed-off-by: Patrick Steinhardt --- builtin/config.c | 3 ++- t/t1300-config.sh | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/config.c b/builtin/config.c index f50c11df575..6708d91814a 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -919,7 +919,8 @@ static int cmd_config_get(int argc, const char **argv, const char *prefix, location_options_init(&location_opts, prefix); display_options_init(&display_opts); - setup_auto_pager("config", 1); + if (display_opts.type != TYPE_COLOR) + setup_auto_pager("config", 1); if (url) ret = get_urlmatch(&location_opts, &display_opts, argv[0], url); diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 3cf5d17abab..358d6363796 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -9,6 +9,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-terminal.sh for mode in legacy subcommands do @@ -1099,6 +1100,14 @@ test_expect_success 'get --type=color with default value only' ' test_cmp actual.raw actual-subcommand.raw ' +test_expect_success TTY 'get --type=color does not use a pager' ' + test_config core.pager "echo foobar" && + test_terminal git config get --type=color --default="red" "" >actual.raw && + test_decode_color actual && + echo "" >expect && + test_cmp expect actual +' + test_expect_success 'set --type=color' ' cat >expect <<\EOF && [foo] -- GitLab