[PATCH 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color"

[PATCH 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color"

[Patrick Steinhardt]

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.

Thanks!

Patrick

---
Patrick Steinhardt (5):
      t1300: write test expectations in the test's body
      t1300: small style fixups
      builtin/config: do not die in `get_color()`
      builtin/config: special-case retrieving colors without a key
      builtin/config: do not spawn pager when printing color codes

 builtin/config.c  |  20 +++-
 t/t1300-config.sh | 344 +++++++++++++++++++++++++++---------------------------
 2 files changed, 185 insertions(+), 179 deletions(-)


---
base-commit: ab427cd991100e94792fce124b0934135abdea4b
change-id: 20250911-pks-config-color-e5b8a213e895

[PATCH 1/5] t1300: write test expectations in the test's body

[Patrick Steinhardt]

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.

Note that there are two exceptions that we don't convert. In both of
these cases the expectation is reused across multiple tests, and thus a
conversion where we'd move that into the first test that uses the
expectation would be invalid. Those are simply left as-is for now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 290 ++++++++++++++++++++++++------------------------------
 1 file changed, 129 insertions(+), 161 deletions(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index f856821839..bde9bda234 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 &&
@@ -214,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
@@ -265,14 +266,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 +294,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 <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'multiple unset is correct' '
 	test_cmp expect .git/config
 '
 
@@ -318,37 +319,38 @@ test_expect_success '--replace-all' '
 	git config ${mode_replace_all} beta.haha gamma
 '
 
-cat > expect << EOF
+test_expect_success 'all replaced' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = gamma
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'all replaced' '
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = alpha
 [nextSection] noNewline = ouch
 EOF
-test_expect_success 'really mean test' '
+
 	git config ${mode_set} beta.haha alpha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -356,7 +358,6 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'really really mean test' '
 	git config ${mode_set} nextsection.nonewline wow &&
 	test_cmp expect .git/config
 '
@@ -365,23 +366,24 @@ test_expect_success 'get value' '
 	test_cmp_config alpha beta.haha
 '
 
-cat > expect << EOF
+test_expect_success 'unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'unset' '
 	git config ${mode_unset} beta.haha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'multivar' '
+	cat  >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -389,7 +391,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar' '
 	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
 	test_cmp expect .git/config
 '
@@ -415,9 +416,10 @@ test_expect_success 'multi-valued get-all returns all' '
 	test_cmp expect actual
 '
 
-cat > expect << EOF
+test_expect_success 'multivar replace' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -425,7 +427,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow3
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar replace' '
 	git config nextsection.nonewline "wow3" "wow$" &&
 	test_cmp expect .git/config
 '
@@ -438,17 +439,16 @@ test_expect_success 'invalid unset' '
 	test_must_fail git config ${mode_unset} somesection.nonewline
 '
 
-cat > expect << EOF
+test_expect_success 'multivar unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar unset' '
 	case "$mode" in
 	legacy)
 		git config --unset nextsection.nonewline "wow3$";;
@@ -466,9 +466,10 @@ test_expect_success 'hierarchical section' '
 	git config Version.1.2.3eX.Alpha beta
 '
 
-cat > expect << EOF
+test_expect_success 'hierarchical section value' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -479,19 +480,16 @@ noIndent= sillyValue ; 'nother silly comment
 [Version "1.2.3eX"]
 	Alpha = beta
 EOF
-
-test_expect_success 'hierarchical section value' '
 	test_cmp expect .git/config
 '
 
-cat > 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 +498,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 +552,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 +593,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 +614,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 +629,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 +661,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 +715,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 +726,6 @@ cat > expect << EOF
 [branch "drei"]
 weird
 EOF
-
-test_expect_success 'rename succeeded' '
 	test_cmp expect .git/config
 '
 
@@ -753,7 +737,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 +750,6 @@ weird
 [branch "zwei"]
 	z = 1
 EOF
-
-test_expect_success 'rename succeeded' '
 	test_cmp expect .git/config
 '
 
@@ -816,32 +799,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 +865,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 +901,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 +913,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 +924,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 +968,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 +980,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 +991,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 +1009,23 @@ then
 	test_set_prereq HOMEVAR
 fi
 
-cat >expect <<EOF
-$HOME/
-/dev/null
-foo~
-EOF
-
 test_expect_success HOMEVAR 'get --path' '
+	cat >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 +1082,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 +1102,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 +1135,12 @@ inued
 inued"
 EOF
 
-cat > expect <<\EOF
+test_expect_success 'value continued on next line' '
+	cat >expect <<\EOF &&
 section.continued=continued
 section.noncont=not continued
 section.quotecont=cont;inued
 EOF
-
-test_expect_success 'value continued on next line' '
 	git config ${mode_prefix}list > result &&
 	test_cmp expect result
 '

-- 
2.51.0.450.g87641ccf93.dirty

Re: [PATCH 1/5] t1300: write test expectations in the test's body

[Kristoffer Haugsbakk]

On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> 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.
>
> Note that there are two exceptions that we don't convert. In both of
> these cases the expectation is reused across multiple tests, and thus a
> conversion where we'd move that into the first test that uses the
> expectation would be invalid. Those are simply left as-is for now.

This is just a suggestion (which everything is):

    Note that there are two exceptions that we leave as-is for now since
    they are reused across tests.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t1300-config.sh | 290 ++++++++++++++++++++++++------------------------------
>  1 file changed, 129 insertions(+), 161 deletions(-)
>
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh

For the following I looked mostly at a color-move etc. diff as well as
the commit.

> index f856821839..bde9bda234 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' '

Ok.  Correct.  But I see that you are also overall doing a sort of
normalization.

• Remove-tabs if it makes sense
• No variable expansion if it makes sense

And here `<<\EOF` would work too.  Or is that worse style?

I will keep mentioning this throughout the rest.

>  	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' '

Ok.  But same as previous.

>  	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' '

Ditto.

>  	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' '

Ditto.

>  	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' '

Ditto.

>  	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 &&
> @@ -214,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
> +'

Okay.  You normalize the one line to

    test_expect_success <name> '
         <body>
    '

The same kind of change done in the next patch `t1300: small
style fixups`.

>
>  test_expect_success 'find mixed-case key by canonical name' '
>  	test_cmp_config Second sections.whatever
> @@ -265,14 +266,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
> +'

Correct.  You eliminate tabs and retain `\`.

>
>  cat > .git/config << EOF
>  [beta] ; silly comment # another comment
> @@ -292,16 +294,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 <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
>  [nextSection] noNewline = ouch
>  EOF
> -
> -test_expect_success 'multiple unset is correct' '
>  	test_cmp expect .git/config
>  '

Correct.  You replaced `'` with `${SQ}` (I assume “single quote”) which
is probably good style in general in order to avoid toothpicks.

>
> @@ -318,37 +319,38 @@ test_expect_success '--replace-all' '
>  	git config ${mode_replace_all} beta.haha gamma
>  '
>
> -cat > expect << EOF
> +test_expect_success 'all replaced' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
>  	haha = gamma
>  [nextSection] noNewline = ouch
>  EOF
> -
> -test_expect_success 'all replaced' '
>  	test_cmp expect .git/config
>  '

Correct.

>
> -cat > expect << EOF
> +test_expect_success 'really mean test' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
>  	haha = alpha
>  [nextSection] noNewline = ouch
>  EOF
> -test_expect_success 'really mean test' '
> +

New blank line.  That can be dropped.

>  	git config ${mode_set} beta.haha alpha &&
>  	test_cmp expect .git/config
>  '
>
> -cat > expect << EOF
> +test_expect_success 'really really mean test' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
> @@ -356,7 +358,6 @@ noIndent= sillyValue ; 'nother silly comment
>  [nextSection]
>  	nonewline = wow
>  EOF
> -test_expect_success 'really really mean test' '
>  	git config ${mode_set} nextsection.nonewline wow &&
>  	test_cmp expect .git/config
>  '

Huh, the similarity between “really” and “really really” confused me.
But correct.

> @@ -365,23 +366,24 @@ test_expect_success 'get value' '
>  	test_cmp_config alpha beta.haha
>  '
>
> -cat > expect << EOF
> +test_expect_success 'unset' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
>  [nextSection]
>  	nonewline = wow
>  EOF
> -test_expect_success 'unset' '
>  	git config ${mode_unset} beta.haha &&
>  	test_cmp expect .git/config
>  '

Correct.

>
> -cat > expect << EOF
> +test_expect_success 'multivar' '
> +	cat  >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
> @@ -389,7 +391,6 @@ noIndent= sillyValue ; 'nother silly comment
>  	nonewline = wow
>  	NoNewLine = wow2 for me
>  EOF
> -test_expect_success 'multivar' '
>  	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
>  	test_cmp expect .git/config
>  '
> @@ -415,9 +416,10 @@ test_expect_success 'multi-valued get-all returns all' '
>  	test_cmp expect actual
>  '

Correct.

>
> -cat > expect << EOF
> +test_expect_success 'multivar replace' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
> @@ -425,7 +427,6 @@ noIndent= sillyValue ; 'nother silly comment
>  	nonewline = wow3
>  	NoNewLine = wow2 for me
>  EOF
> -test_expect_success 'multivar replace' '
>  	git config nextsection.nonewline "wow3" "wow$" &&
>  	test_cmp expect .git/config
>  '

Correct.

> @@ -438,17 +439,16 @@ test_expect_success 'invalid unset' '
>  	test_must_fail git config ${mode_unset} somesection.nonewline
>  '
>
> -cat > expect << EOF
> +test_expect_success 'multivar unset' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
>  [nextSection]
>  	NoNewLine = wow2 for me
>  EOF
> -
> -test_expect_success 'multivar unset' '
>  	case "$mode" in
>  	legacy)
>  		git config --unset nextsection.nonewline "wow3$";;

Correct.

> @@ -466,9 +466,10 @@ test_expect_success 'hierarchical section' '
>  	git config Version.1.2.3eX.Alpha beta
>  '
>
> -cat > expect << EOF
> +test_expect_success 'hierarchical section value' '
> +	cat >expect <<EOF &&
>  [beta] ; silly comment # another comment
> -noIndent= sillyValue ; 'nother silly comment
> +noIndent= sillyValue ; ${SQ}nother silly comment
>
>  # empty line
>  		; comment
> @@ -479,19 +480,16 @@ noIndent= sillyValue ; 'nother silly comment
>  [Version "1.2.3eX"]
>  	Alpha = beta
>  EOF
> -
> -test_expect_success 'hierarchical section value' '
>  	test_cmp expect .git/config
>  '

Correct.

>
> -cat > 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
>  '

Correct.  Removing tabs now that you can.  Also no variable expansion.

> @@ -500,44 +498,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
>  '

Correct.  But you can use `<<-\EOF`.

>
> -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
>  '

Ditto.

>
> -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
>  '

Ditto.

>
> -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

Ditto.

> @@ -558,37 +552,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
>  '

Correct.

>
> -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
>  '

Correct.

>
> -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
>  '

Correct.

>
> -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
>  '

Correct.

>
> -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
>  '

Here you should use `>expect`.

> @@ -604,19 +593,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
>  '

You can use `<<\EOF`.

>
> -cat > expect << EOF
> +test_expect_success 'new variable inserts into proper section' '
> +	cat >expect <<EOF &&
>  [a.b]
>  	c = d
>  [a]
> @@ -625,8 +614,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

Correct.  You can use `<<\EOF`.

> @@ -642,11 +629,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

Ditto.

>  	GIT_CONFIG=other-config git config ${mode_prefix}list >output &&
>  	test_cmp expect output
>  '
> @@ -675,14 +661,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
>  '

Correct.  Here you do `<< EOF` → `<<\EOF`.

> @@ -730,7 +715,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 +726,6 @@ cat > expect << EOF
>  [branch "drei"]
>  weird
>  EOF
> -
> -test_expect_success 'rename succeeded' '
>  	test_cmp expect .git/config
>  '

Ditto.

(That four tests are named `rename succeeded` is a bit disorienting.)

>
> @@ -753,7 +737,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 +750,6 @@ weird
>  [branch "zwei"]
>  	z = 1
>  EOF
> -
> -test_expect_success 'rename succeeded' '
>  	test_cmp expect .git/config
>  '

Ditto.

>
> @@ -816,32 +799,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
>  '

Ditto.

>
> -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
> -
>  '

Ditto.  And normalized blank lines, that’s good.

>
>  test_expect_success numbers '
> @@ -885,19 +865,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 &&

Correct.

> @@ -923,7 +901,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 +913,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 &&

Correct.  But add a space? (>expect<<\EOF)

> @@ -948,15 +924,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 &&

Correct.

> @@ -994,7 +968,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 +980,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 &&

Correct.

I tried with here `<<-\EOF` but that made something fail. :)

> @@ -1018,14 +991,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" &&

Correct.

> @@ -1037,25 +1009,23 @@ then
>  	test_set_prereq HOMEVAR
>  fi
>
> -cat >expect <<EOF
> -$HOME/
> -/dev/null
> -foo~
> -EOF
> -
>  test_expect_success HOMEVAR 'get --path' '
> +	cat >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
>  '

Correct.

>
> -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 \

Correct.

> @@ -1112,12 +1082,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

Correct.  You can use `<<\EOF`.

> @@ -1133,14 +1102,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 " &&

Correct.  You can use `<<\EOF`.

> @@ -1166,13 +1135,12 @@ inued
>  inued"
>  EOF
>
> -cat > expect <<\EOF
> +test_expect_success 'value continued on next line' '
> +	cat >expect <<\EOF &&
>  section.continued=continued
>  section.noncont=not continued
>  section.quotecont=cont;inued
>  EOF
> -
> -test_expect_success 'value continued on next line' '
>  	git config ${mode_prefix}list > result &&
>  	test_cmp expect result
>  '

Correct.  You can use

	cat >expect <<-\EOF &&
	section.continued=continued
	section.noncont=not continued
	section.quotecont=cont;inued
	EOF

>
> --
> 2.51.0.450.g87641ccf93.dirty

Re: [PATCH 1/5] t1300: write test expectations in the test's body

[Patrick Steinhardt]

On Thu, Sep 11, 2025 at 06:50:29PM +0200, Kristoffer Haugsbakk wrote:
> On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> > 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.
> >
> > Note that there are two exceptions that we don't convert. In both of
> > these cases the expectation is reused across multiple tests, and thus a
> > conversion where we'd move that into the first test that uses the
> > expectation would be invalid. Those are simply left as-is for now.
> 
> This is just a suggestion (which everything is):
> 
>     Note that there are two exceptions that we leave as-is for now since
>     they are reused across tests.

Yup, that reads way better.

> > index f856821839..bde9bda234 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' '
> 
> Ok.  Correct.  But I see that you are also overall doing a sort of
> normalization.
> 
> • Remove-tabs if it makes sense
> • No variable expansion if it makes sense
> 
> And here `<<\EOF` would work too.  Or is that worse style?
> 
> I will keep mentioning this throughout the rest.

No, it's not. I already did it sometimes, but not consistently. I'll
mention this in the commit message and adapt as possible.

> > @@ -214,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
> > +'
> 
> Okay.  You normalize the one line to
> 
>     test_expect_success <name> '
>          <body>
>     '
> 
> The same kind of change done in the next patch `t1300: small
> style fixups`.

True, will move into the next patch.

Thanks for going through all of these!

Patrick

[PATCH 2/5] t1300: small style fixups

[Patrick Steinhardt]

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 <ps@pks.im>
---
 t/t1300-config.sh | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index bde9bda234..9c405e9532 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -458,9 +458,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
@@ -493,6 +497,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
@@ -890,16 +895,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 &&
@@ -1002,7 +1008,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
@@ -1120,10 +1127,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]
@@ -1333,7 +1343,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.
@@ -1352,7 +1361,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.

-- 
2.51.0.450.g87641ccf93.dirty

[PATCH 3/5] builtin/config: do not die in `get_color()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 59fb113b07..afd48bfa51 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);

-- 
2.51.0.450.g87641ccf93.dirty

[PATCH 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

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" ""

What this command is supposed to do is to not parse any configuration
key at all. 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 with 4e51389000 (builtin/config: introduce
"get" subcommand, 2024-05-06), where we introduced the new "get"
subcommand to retrieve configuration values. The preimage of that commit
used `git config --get-color "" "reset"` instead, which still works
nowadays.

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, but 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
has provided a value, then we call `get_color()` directly. Do so to
make the documented command work as expected.

[1]: <aI+oQvQgnNtC6DVw@szeder.dev>

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 afd48bfa51..f50c11df57 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 9c405e9532..40f170cf40 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1084,11 +1084,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.raw >actual &&
 	echo "<RED>" >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.raw >actual &&
+	echo "<RED>" >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]

-- 
2.51.0.450.g87641ccf93.dirty

Re: [PATCH 4/5] builtin/config: special-case retrieving colors without a key

[Kristoffer Haugsbakk]

On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> 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

Okay.  This is simple to find with a `color` search.

> ANSI color escape sequence to reset the colors to normal we recommend
> the following command:
>
>     $ git config get --type=color --default="reset" ""
>
> What this command is supposed to do is to not parse any configuration
> key at all.

Or

    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 with 4e51389000 (builtin/config: introduce

IMO s/with/in/ ?

> "get" subcommand, 2024-05-06), where we introduced the new "get"

nit: s/introduced the new/introduced the/

> subcommand to retrieve configuration values. The preimage of that commit
> used `git config --get-color "" "reset"` instead, which still works
> nowadays.

nit: s/still works nowadays/still works/

>
> This use case is really quite specific to parsing colors, as it wouldn't

s/use case/use-case/

> 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, but we instead parse the value into an
> ANSI escape sequence.

Two “but”?  Maybe

    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

s/use case/use-case/

Like special-case.

>
> 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
> has provided a value, then we call `get_color()` directly. Do so to
> make the documented command work as expected.

In my opinion this is more difficult to read without an Oxford comma.
A bullet list could break up the serial comma and the comma used to
separate the “then” subclause.

    use-case:

    - if the provided config key is empty;

    - the user is asking for a color code; and

    - the user has provided a value,

    then we ...

In any case: I think a colon generally means that semicolon will be used
instead of serial comma.

>
> [1]: <aI+oQvQgnNtC6DVw@szeder.dev>
>
> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>[snip too technical diff]

Re: [PATCH 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

On Thu, Sep 11, 2025 at 06:48:45PM +0200, Kristoffer Haugsbakk wrote:
> On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> > This use case is really quite specific to parsing colors, as it wouldn't
> 
> s/use case/use-case/

I used to say "usecase", but eventually got corrected by one of our
technical writers at GitLab that this is "use case". Wikipedia seems to
agree [1].

[1]: https://en.wikipedia.org/wiki/Use_case

> > 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, but we instead parse the value into an
> > ANSI escape sequence.
> 
> Two “but”?  Maybe
> 
>     But with `--type=color` we don't return the value directly; we
>     instead parse the value into an ANSI escape sequence.

That reads nicer, yup.

> > 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
> > has provided a value, then we call `get_color()` directly. Do so to
> > make the documented command work as expected.
> 
> In my opinion this is more difficult to read without an Oxford comma.
> A bullet list could break up the serial comma and the comma used to
> separate the “then” subclause.
> 
>     use-case:
> 
>     - if the provided config key is empty;
> 
>     - the user is asking for a color code; and
> 
>     - the user has provided a value,
> 
>     then we ...
> 
> In any case: I think a colon generally means that semicolon will be used
> instead of serial comma.

Yup, good suggestion.

Patrick

[PATCH 5/5] builtin/config: do not spawn pager when printing color codes

[Patrick Steinhardt]

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 though, which means that the
string may instead be written to the pager command. This is of course
quite nonsensical: there shouldn't be any use case where the color code
should end up in the pager instead of in the TTY.

Fix this by disabling the pager in case the user is asking us to print
color sequences.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 f50c11df57..6708d91814 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 40f170cf40..26111c175c 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
@@ -1100,6 +1101,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.raw >actual &&
+	echo "<RED>" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'set --type=color' '
 	cat >expect <<EOF &&
 [foo]

-- 
2.51.0.450.g87641ccf93.dirty

Re: [PATCH 5/5] builtin/config: do not spawn pager when printing color codes

[Kristoffer Haugsbakk]

On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
> 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 though, which means that the

Double “though”.

> string may instead be written to the pager command. This is of course
> quite nonsensical: there shouldn't be any use case where the color code

IMO s/:/;/

s/use case/use-case/

> should end up in the pager instead of in the TTY.
>
> Fix this by disabling the pager in case the user is asking us to print
> color sequences.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>[snip the diff]

[PATCH v2 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color"

[Patrick Steinhardt]

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

Thanks!

Patrick

---
Patrick Steinhardt (5):
      t1300: write test expectations in the test's body
      t1300: small style fixups
      builtin/config: do not die in `get_color()`
      builtin/config: special-case retrieving colors without a key
      builtin/config: do not spawn pager when printing color codes

 builtin/config.c  |  20 +++-
 t/t1300-config.sh | 349 +++++++++++++++++++++++++++---------------------------
 2 files changed, 187 insertions(+), 182 deletions(-)

Range-diff versus v1:

1:  2920521b26 ! 1:  e58033502e t1300: write test expectations in the test's body
    @@ Commit message
         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.
    +    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 don't convert. In both of
    -    these cases the expectation is reused across multiple tests, and thus a
    -    conversion where we'd move that into the first test that uses the
    -    expectation would be invalid. Those are simply left as-is for now.
    +    Note that there are two exceptions that we leave as-is for now since
    +    they are reused across tests.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ t/t1300-config.sh: test_expect_success 'clear default config' '
      
     -cat > expect << EOF
     +test_expect_success 'initial' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [section]
      	penguin = little blue
      EOF
    @@ t/t1300-config.sh: test_expect_success 'clear default config' '
      
     -cat > expect << EOF
     +test_expect_success 'mixed case' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [section]
      	penguin = little blue
      	Movie = BadPhysics
    @@ t/t1300-config.sh: test_expect_success 'clear default config' '
      
     -cat > expect << EOF
     +test_expect_success 'similar section' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [section]
      	penguin = little blue
      	Movie = BadPhysics
    @@ t/t1300-config.sh: test_expect_success 'clear default config' '
      
     -cat > expect << EOF
     +test_expect_success 'uppercase section' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [section]
      	penguin = little blue
      	Movie = BadPhysics
    @@ t/t1300-config.sh: test_expect_success 'replace with non-match (actually matchin
      
     -cat > expect << EOF
     +test_expect_success 'append comments' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [section]
      	Movie = BadPhysics
      	UPPERCASE = true
    @@ t/t1300-config.sh: cat > expect << EOF
      	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 &&
    -@@ t/t1300-config.sh: 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
     @@ t/t1300-config.sh: test_expect_success 'unset with cont. lines' '
      	git config ${mode_unset} beta.baz
      '
    @@ t/t1300-config.sh: test_expect_success '--replace-all' '
      [nextSection] noNewline = ouch
      EOF
     -test_expect_success 'really mean test' '
    -+
      	git config ${mode_set} beta.haha alpha &&
      	test_cmp expect .git/config
      '
    @@ t/t1300-config.sh: test_expect_success '--list without repo produces empty outpu
     -EOF
     -
      test_expect_success '--name-only --list' '
    -+	cat >expect <<-EOF &&
    ++	cat >expect <<-\EOF &&
     +	beta.noindent
     +	nextsection.nonewline
     +	123456.a123
    @@ t/t1300-config.sh: test_expect_success '--list without repo produces empty outpu
     -EOF
     -
      test_expect_success '--get-regexp' '
    -+	cat >expect <<-EOF &&
    ++	cat >expect <<-\EOF &&
     +	beta.noindent sillyValue
     +	nextsection.nonewline wow2 for me
     +	EOF
    @@ t/t1300-config.sh: test_expect_success '--list without repo produces empty outpu
     -EOF
     -
      test_expect_success '--name-only --get-regexp' '
    -+	cat >expect <<-EOF &&
    ++	cat >expect <<-\EOF &&
     +	beta.noindent
     +	nextsection.nonewline
     +	EOF
    @@ t/t1300-config.sh: test_expect_success '--list without repo produces empty outpu
     -EOF
     -
      test_expect_success '--add' '
    -+	cat >expect <<-EOF &&
    ++	cat >expect <<-\EOF &&
     +	wow2 for me
     +	wow4 for you
     +	EOF
    @@ t/t1300-config.sh: test_expect_success 'get variable with empty value' '
     -echo false > expect
     -
      test_expect_success 'get bool variable with empty value' '
    -+	echo false > expect &&
    ++	echo false >expect &&
      	git config --bool emptyvalue.variable > output &&
      	test_cmp expect output
      '
    @@ t/t1300-config.sh: cat > .git/config << EOF
      
     -cat > expect << EOF
     +test_expect_success 'new section is partial match of another' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [a.b]
      	c = d
      [a]
    @@ t/t1300-config.sh: cat > .git/config << EOF
      
     -cat > expect << EOF
     +test_expect_success 'new variable inserts into proper section' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [a.b]
      	c = d
      [a]
    @@ t/t1300-config.sh: cat > other-config << EOF
     -EOF
     -
      test_expect_success 'alternative GIT_CONFIG' '
    -+	cat >expect <<-EOF &&
    ++	cat >expect <<-\EOF &&
     +	ein.bahn=strasse
     +	EOF
      	GIT_CONFIG=other-config git config ${mode_prefix}list >output &&
    @@ t/t1300-config.sh: test_expect_success 'invalid bool (set)' '
      
     -cat > expect <<\EOF
     +test_expect_success 'set --bool' '
    -+	cat >expect<<\EOF &&
    ++	cat >expect <<\EOF &&
      [bool]
      	true1 = true
      	true2 = true
    @@ t/t1300-config.sh: test_expect_success 'get --type=color' '
      
     -cat >expect << EOF
     +test_expect_success 'set --type=color' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [foo]
      	color = red
      EOF
    @@ t/t1300-config.sh: test_expect_success 'set --type=color barfs on non-color' '
      
     -cat > expect << EOF
     +test_expect_success 'quoting' '
    -+	cat >expect <<EOF &&
    ++	cat >expect <<\EOF &&
      [quote]
      	leading = " test"
      	ending = "test "
    @@ t/t1300-config.sh: inued
      EOF
      
     -cat > expect <<\EOF
    -+test_expect_success 'value continued on next line' '
    -+	cat >expect <<\EOF &&
    - section.continued=continued
    - section.noncont=not continued
    - section.quotecont=cont;inued
    - EOF
    +-section.continued=continued
    +-section.noncont=not continued
    +-section.quotecont=cont;inued
    +-EOF
     -
    --test_expect_success 'value continued on next line' '
    + 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
      '
2:  a6a38b98c2 ! 2:  c37605959e t1300: small style fixups
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## t/t1300-config.sh ##
    +@@ t/t1300-config.sh: 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
     @@ t/t1300-config.sh: EOF
      	test_cmp expect .git/config
      '
    @@ t/t1300-config.sh: test_expect_success bool '
     +'
      
      test_expect_success 'set --bool' '
    - 	cat >expect<<\EOF &&
    + 	cat >expect <<\EOF &&
     @@ t/t1300-config.sh: EOF
      	git config --path path.home "~/" &&
      	git config --path path.normal "/dev/null" &&
3:  b9fe7190a1 = 3:  d88e317762 builtin/config: do not die in `get_color()`
4:  1cd6acd0e9 ! 4:  6dfb71e7ce builtin/config: special-case retrieving colors without a key
    @@ Commit message
     
             $ git config get --type=color --default="reset" ""
     
    -    What this command is supposed to do is to not parse any configuration
    -    key at all. Instead, it is expected to parse the "reset" default value
    -    and turn it into a proper ANSI color escape sequence.
    +    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 with 4e51389000 (builtin/config: introduce
    -    "get" subcommand, 2024-05-06), where we introduced the new "get"
    -    subcommand to retrieve configuration values. The preimage of that commit
    -    used `git config --get-color "" "reset"` instead, which still works
    -    nowadays.
    +    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, but we instead parse the value into an
    -    ANSI escape sequence.
    +    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
    -    has provided a value, then we call `get_color()` directly. Do so to
    -    make the documented command work as expected.
    +    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]: <aI+oQvQgnNtC6DVw@szeder.dev>
     
    @@ t/t1300-config.sh: test_expect_success 'get --type=color' '
     +'
     +
      test_expect_success 'set --type=color' '
    - 	cat >expect <<EOF &&
    + 	cat >expect <<\EOF &&
      [foo]
5:  46fc98e1ec ! 5:  ad443d744f builtin/config: do not spawn pager when printing color codes
    @@ Commit message
         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 though, which means that the
    -    string may instead be written to the pager command. This is of course
    -    quite nonsensical: there shouldn't be any use case where the color code
    -    should end up in the pager instead of in the TTY.
    +    Right now though we set up the auto-pager, which means that the string
    +    may instead be written to the pager command. This is of course quite
    +    nonsensical; there shouldn't be any use case where the color code should
    +    end up in the pager instead of in the TTY.
     
         Fix this by disabling the pager in case the user is asking us to print
         color sequences.
    @@ t/t1300-config.sh: test_expect_success 'get --type=color with default value only
     +'
     +
      test_expect_success 'set --type=color' '
    - 	cat >expect <<EOF &&
    + 	cat >expect <<\EOF &&
      [foo]

---
base-commit: ab427cd991100e94792fce124b0934135abdea4b
change-id: 20250911-pks-config-color-e5b8a213e895

[PATCH v2 1/5] t1300: write test expectations in the test's body

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 f856821839..538f2c9b8a 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 <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'multiple unset is correct' '
 	test_cmp expect .git/config
 '
 
@@ -318,37 +317,37 @@ test_expect_success '--replace-all' '
 	git config ${mode_replace_all} beta.haha gamma
 '
 
-cat > expect << EOF
+test_expect_success 'all replaced' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = gamma
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'all replaced' '
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = alpha
 [nextSection] noNewline = ouch
 EOF
-test_expect_success 'really mean test' '
 	git config ${mode_set} beta.haha alpha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -356,7 +355,6 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'really really mean test' '
 	git config ${mode_set} nextsection.nonewline wow &&
 	test_cmp expect .git/config
 '
@@ -365,23 +363,24 @@ test_expect_success 'get value' '
 	test_cmp_config alpha beta.haha
 '
 
-cat > expect << EOF
+test_expect_success 'unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'unset' '
 	git config ${mode_unset} beta.haha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'multivar' '
+	cat  >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -389,7 +388,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar' '
 	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
 	test_cmp expect .git/config
 '
@@ -415,9 +413,10 @@ test_expect_success 'multi-valued get-all returns all' '
 	test_cmp expect actual
 '
 
-cat > expect << EOF
+test_expect_success 'multivar replace' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -425,7 +424,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow3
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar replace' '
 	git config nextsection.nonewline "wow3" "wow$" &&
 	test_cmp expect .git/config
 '
@@ -438,17 +436,16 @@ test_expect_success 'invalid unset' '
 	test_must_fail git config ${mode_unset} somesection.nonewline
 '
 
-cat > expect << EOF
+test_expect_success 'multivar unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar unset' '
 	case "$mode" in
 	legacy)
 		git config --unset nextsection.nonewline "wow3$";;
@@ -466,9 +463,10 @@ test_expect_success 'hierarchical section' '
 	git config Version.1.2.3eX.Alpha beta
 '
 
-cat > expect << EOF
+test_expect_success 'hierarchical section value' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -479,19 +477,16 @@ noIndent= sillyValue ; 'nother silly comment
 [Version "1.2.3eX"]
 	Alpha = beta
 EOF
-
-test_expect_success 'hierarchical section value' '
 	test_cmp expect .git/config
 '
 
-cat > 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 <<EOF
-$HOME/
-/dev/null
-foo~
-EOF
-
 test_expect_success HOMEVAR 'get --path' '
+	cat >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
 '

-- 
2.51.0.450.g87641ccf93.dirty

[PATCH v2 2/5] t1300: small style fixups

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 538f2c9b8a..6d1015acfd 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.

-- 
2.51.0.450.g87641ccf93.dirty

[PATCH v2 3/5] builtin/config: do not die in `get_color()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 59fb113b07..afd48bfa51 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);

-- 
2.51.0.450.g87641ccf93.dirty

[PATCH v2 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

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]: <aI+oQvQgnNtC6DVw@szeder.dev>

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 afd48bfa51..f50c11df57 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 6d1015acfd..3cf5d17aba 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.raw >actual &&
 	echo "<RED>" >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.raw >actual &&
+	echo "<RED>" >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]

-- 
2.51.0.450.g87641ccf93.dirty

Re: [PATCH v2 4/5] builtin/config: special-case retrieving colors without a key

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

Yup, the UI is still ugly because this is not config at all X-<.
Only because we lack "I have a color name.  Please parse it into
ANSI sequence" command, we are abusing the "git config" command and
using "--default" as "here is that color name I am asking about"
argument.  Even if we were to add a more intuitive "parse --color"
command, the (ab)use of "git config" for this purpose has been with
us for the many years and cannot be dropped--and under that constraints,
I agree that this patch is the best we could do.

Thanks for working on this.

[PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes

[Patrick Steinhardt]

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 instead be written to the pager command. This is of course quite
nonsensical; there shouldn't be any use case where the color code should
end up in the pager instead of in the TTY.

Fix this by disabling the pager in case the user is asking us to print
color sequences.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 f50c11df57..6708d91814 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 3cf5d17aba..358d636379 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.raw >actual &&
+	echo "<RED>" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'set --type=color' '
 	cat >expect <<\EOF &&
 [foo]

-- 
2.51.0.450.g87641ccf93.dirty

Re: [PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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 instead be written to the pager command. This is of course quite
> nonsensical; there shouldn't be any use case where the color code should
> end up in the pager instead of in the TTY.
>
> Fix this by disabling the pager in case the user is asking us to print
> color sequences.

I am of two minds.  Part of me obviously agrees that it is more
straight forward with this change.  But it may 

An interactive user experimenting while writing their own script
might say something like

 $ git config --type=color --default="reverse red" n.n

If the command emitted directly to the terminal, then everything
they type from then on will be bloody red, but the pager protects
them from such an accident.  Instead, they are forced to say

 $ C=$(git config get --type=color --default="reverse red" n.n)
 $ R=$(git config get --type=color --default="reset" n.n)
 $ echo "So$C Bloody ${R}Red"

but these are likely what they would be writing in their script
anyway, so...

Re: [PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes

[Kristoffer Haugsbakk]

On Mon, Sep 15, 2025, at 19:28, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>[snip]
>> Fix this by disabling the pager in case the user is asking us to print
>> color sequences.
>
> I am of two minds.  Part of me obviously agrees that it is more
> straight forward with this change.  But it may 

Did you drop something here or is it covered by the next paragraph?

>
> An interactive user experimenting while writing their own script
> might say something like
>
>  $ git config --type=color --default="reverse red" n.n
>
>[snip]

Re: [PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes

[Patrick Steinhardt]

On Mon, Sep 15, 2025 at 10:28:23AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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 instead be written to the pager command. This is of course quite
> > nonsensical; there shouldn't be any use case where the color code should
> > end up in the pager instead of in the TTY.
> >
> > Fix this by disabling the pager in case the user is asking us to print
> > color sequences.
> 
> I am of two minds.  Part of me obviously agrees that it is more
> straight forward with this change.  But it may 
> 
> An interactive user experimenting while writing their own script
> might say something like
> 
>  $ git config --type=color --default="reverse red" n.n
> 
> If the command emitted directly to the terminal, then everything
> they type from then on will be bloody red, but the pager protects
> them from such an accident.  Instead, they are forced to say
> 
>  $ C=$(git config get --type=color --default="reverse red" n.n)
>  $ R=$(git config get --type=color --default="reset" n.n)
>  $ echo "So$C Bloody ${R}Red"
> 
> but these are likely what they would be writing in their script
> anyway, so...

True. That being said, I'm mostly trying to emulate the old behaviour
that we had in `git config --get-color`. We have the following condition
there:

	/*
	 * The following actions may produce more than one line of output and
	 * should therefore be paged.
	 */
	if (actions & (ACTION_LIST | ACTION_GET_ALL | ACTION_GET_REGEXP | ACTION_GET_URLMATCH))
		setup_auto_pager("config", 1);

`ACTION_GET_COLOR` is not part of this condition, so we wouldn't set up
the auto pager there, either. So I think it's sensible to match that
behaviour so that the new command really is a drop-in replacement for
the old one.

I should probably clarify the commit message.

Patrick

[PATCH v3 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color"

[Patrick Steinhardt]

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

---
Patrick Steinhardt (5):
      t1300: write test expectations in the test's body
      t1300: small style fixups
      builtin/config: do not die in `get_color()`
      builtin/config: special-case retrieving colors without a key
      builtin/config: do not spawn pager when printing color codes

 builtin/config.c  |  20 +++-
 t/t1300-config.sh | 349 +++++++++++++++++++++++++++---------------------------
 2 files changed, 187 insertions(+), 182 deletions(-)

Range-diff versus v2:

1:  e61278d7c5 = 1:  fdd1711881 t1300: write test expectations in the test's body
2:  e6f1ea5283 = 2:  8e1e05d9e1 t1300: small style fixups
3:  6f8257aeb4 = 3:  1ef6272e64 builtin/config: do not die in `get_color()`
4:  27a8ab34b0 = 4:  5dcf8c6656 builtin/config: special-case retrieving colors without a key
5:  259600c32a ! 5:  05d5022c1b builtin/config: do not spawn pager when printing color codes
    @@ Commit message
         to reuse the same colors as Git.
     
         Right now though we set up the auto-pager, which means that the string
    -    may instead be written to the pager command. This is of course quite
    -    nonsensical; there shouldn't be any use case where the color code should
    -    end up in the pager instead of in the TTY.
    +    may be written to the pager instead of directly to the terminal. This
    +    behaviour is problematic for two reasons:
     
    -    Fix this by disabling the pager in case the user is asking us to print
    -    color sequences.
    +      - 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 <ps@pks.im>
     

---
base-commit: ab427cd991100e94792fce124b0934135abdea4b
change-id: 20250911-pks-config-color-e5b8a213e895

[PATCH v3 1/5] t1300: write test expectations in the test's body

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 f856821839..538f2c9b8a 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 <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'multiple unset is correct' '
 	test_cmp expect .git/config
 '
 
@@ -318,37 +317,37 @@ test_expect_success '--replace-all' '
 	git config ${mode_replace_all} beta.haha gamma
 '
 
-cat > expect << EOF
+test_expect_success 'all replaced' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = gamma
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'all replaced' '
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = alpha
 [nextSection] noNewline = ouch
 EOF
-test_expect_success 'really mean test' '
 	git config ${mode_set} beta.haha alpha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -356,7 +355,6 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'really really mean test' '
 	git config ${mode_set} nextsection.nonewline wow &&
 	test_cmp expect .git/config
 '
@@ -365,23 +363,24 @@ test_expect_success 'get value' '
 	test_cmp_config alpha beta.haha
 '
 
-cat > expect << EOF
+test_expect_success 'unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'unset' '
 	git config ${mode_unset} beta.haha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'multivar' '
+	cat  >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -389,7 +388,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar' '
 	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
 	test_cmp expect .git/config
 '
@@ -415,9 +413,10 @@ test_expect_success 'multi-valued get-all returns all' '
 	test_cmp expect actual
 '
 
-cat > expect << EOF
+test_expect_success 'multivar replace' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -425,7 +424,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow3
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar replace' '
 	git config nextsection.nonewline "wow3" "wow$" &&
 	test_cmp expect .git/config
 '
@@ -438,17 +436,16 @@ test_expect_success 'invalid unset' '
 	test_must_fail git config ${mode_unset} somesection.nonewline
 '
 
-cat > expect << EOF
+test_expect_success 'multivar unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar unset' '
 	case "$mode" in
 	legacy)
 		git config --unset nextsection.nonewline "wow3$";;
@@ -466,9 +463,10 @@ test_expect_success 'hierarchical section' '
 	git config Version.1.2.3eX.Alpha beta
 '
 
-cat > expect << EOF
+test_expect_success 'hierarchical section value' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -479,19 +477,16 @@ noIndent= sillyValue ; 'nother silly comment
 [Version "1.2.3eX"]
 	Alpha = beta
 EOF
-
-test_expect_success 'hierarchical section value' '
 	test_cmp expect .git/config
 '
 
-cat > 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 <<EOF
-$HOME/
-/dev/null
-foo~
-EOF
-
 test_expect_success HOMEVAR 'get --path' '
+	cat >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
 '

-- 
2.51.0.534.gc79095c0ca.dirty

[PATCH v3 2/5] t1300: small style fixups

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 538f2c9b8a..6d1015acfd 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.

-- 
2.51.0.534.gc79095c0ca.dirty

[PATCH v3 3/5] builtin/config: do not die in `get_color()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 59fb113b07..afd48bfa51 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);

-- 
2.51.0.534.gc79095c0ca.dirty

[PATCH v3 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

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]: <aI+oQvQgnNtC6DVw@szeder.dev>

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 afd48bfa51..f50c11df57 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 6d1015acfd..3cf5d17aba 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.raw >actual &&
 	echo "<RED>" >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.raw >actual &&
+	echo "<RED>" >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]

-- 
2.51.0.534.gc79095c0ca.dirty

Re: [PATCH v3 4/5] builtin/config: special-case retrieving colors without a key

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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

"and the user;" -> ";" perhaps?

>
>     - the user has provided a default value,
>
> then we call `get_color()` directly. Do so to make the documented
> command work as expected.

If we are willing to handle this as a special case anyway, I wonder
if it can easily be arranged to take this as a(nother) special case.

    $ git config get --type=color --default="reset"

I.e., instead of (or in addition to) "if the config key is empty",
special case "if the config key is not given", which may be slightly
more intuitive.

But even without it, what is presented is a vast improvement enough
;-)

Re: [PATCH v3 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

On Wed, Sep 17, 2025 at 11:49:18PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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
> 
> "and the user;" -> ";" perhaps?

Oh, yeah, thanks.

> >     - the user has provided a default value,
> >
> > then we call `get_color()` directly. Do so to make the documented
> > command work as expected.
> 
> If we are willing to handle this as a special case anyway, I wonder
> if it can easily be arranged to take this as a(nother) special case.
> 
>     $ git config get --type=color --default="reset"
> 
> I.e., instead of (or in addition to) "if the config key is empty",
> special case "if the config key is not given", which may be slightly
> more intuitive.
> 
> But even without it, what is presented is a vast improvement enough
> ;-)

We probably could, yeah. But it starts to become even weirder than it
already is, so I'd honestly just leave it as-is for now :) I doubt that
there's too many users out there that care about this anyway.

Patrick

Re: [PATCH v3 4/5] builtin/config: special-case retrieving colors without a key

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> If we are willing to handle this as a special case anyway, I wonder
>> if it can easily be arranged to take this as a(nother) special case.
>> 
>>     $ git config get --type=color --default="reset"
>> 
>> I.e., instead of (or in addition to) "if the config key is empty",
>> special case "if the config key is not given", which may be slightly
>> more intuitive.
>> 
>> But even without it, what is presented is a vast improvement enough
>> ;-)
>
> We probably could, yeah. But it starts to become even weirder than it
> already is, so I'd honestly just leave it as-is for now :) I doubt that
> there's too many users out there that care about this anyway.

Yes, unless we can require "--default" to be spelled "--translate-this"
only in that specific use case (which we don't want to), the keyless
case would be even more strange than the empty key case, so I am
happy with what we see in this series.

Thanks.

[PATCH v3 5/5] builtin/config: do not spawn pager when printing color codes

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 f50c11df57..6708d91814 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 3cf5d17aba..358d636379 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.raw >actual &&
+	echo "<RED>" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'set --type=color' '
 	cat >expect <<\EOF &&
 [foo]

-- 
2.51.0.534.gc79095c0ca.dirty

[PATCH v4 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color"

[Patrick Steinhardt]

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

Changes in v4:
  - Fix a commit message grammar bug.
  - Link to v3: https://lore.kernel.org/r/20250918-pks-config-color-v3-0-08ea618cae26@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (5):
      t1300: write test expectations in the test's body
      t1300: small style fixups
      builtin/config: do not die in `get_color()`
      builtin/config: special-case retrieving colors without a key
      builtin/config: do not spawn pager when printing color codes

 builtin/config.c  |  20 +++-
 t/t1300-config.sh | 349 +++++++++++++++++++++++++++---------------------------
 2 files changed, 187 insertions(+), 182 deletions(-)

Range-diff versus v3:

1:  e62108edc8 = 1:  8d873d65de t1300: write test expectations in the test's body
2:  deef91062a = 2:  39eeb35d43 t1300: small style fixups
3:  58940b2cf9 = 3:  ee22438222 builtin/config: do not die in `get_color()`
4:  288d51d1c0 ! 4:  44b1d54527 builtin/config: special-case retrieving colors without a key
    @@ Commit message
     
             - If the provided config key is empty;
     
    -        - the user is asking for a color code and the user; and
    +        - the user is asking for a color code; and
     
             - the user has provided a default value,
     
5:  9a3adf8795 = 5:  b4d72c21f5 builtin/config: do not spawn pager when printing color codes

---
base-commit: ab427cd991100e94792fce124b0934135abdea4b
change-id: 20250911-pks-config-color-e5b8a213e895

[PATCH v4 1/5] t1300: write test expectations in the test's body

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 f856821839..538f2c9b8a 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 <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'multiple unset is correct' '
 	test_cmp expect .git/config
 '
 
@@ -318,37 +317,37 @@ test_expect_success '--replace-all' '
 	git config ${mode_replace_all} beta.haha gamma
 '
 
-cat > expect << EOF
+test_expect_success 'all replaced' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = gamma
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'all replaced' '
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 	haha = alpha
 [nextSection] noNewline = ouch
 EOF
-test_expect_success 'really mean test' '
 	git config ${mode_set} beta.haha alpha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'really really mean test' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -356,7 +355,6 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'really really mean test' '
 	git config ${mode_set} nextsection.nonewline wow &&
 	test_cmp expect .git/config
 '
@@ -365,23 +363,24 @@ test_expect_success 'get value' '
 	test_cmp_config alpha beta.haha
 '
 
-cat > expect << EOF
+test_expect_success 'unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	nonewline = wow
 EOF
-test_expect_success 'unset' '
 	git config ${mode_unset} beta.haha &&
 	test_cmp expect .git/config
 '
 
-cat > expect << EOF
+test_expect_success 'multivar' '
+	cat  >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -389,7 +388,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar' '
 	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
 	test_cmp expect .git/config
 '
@@ -415,9 +413,10 @@ test_expect_success 'multi-valued get-all returns all' '
 	test_cmp expect actual
 '
 
-cat > expect << EOF
+test_expect_success 'multivar replace' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -425,7 +424,6 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow3
 	NoNewLine = wow2 for me
 EOF
-test_expect_success 'multivar replace' '
 	git config nextsection.nonewline "wow3" "wow$" &&
 	test_cmp expect .git/config
 '
@@ -438,17 +436,16 @@ test_expect_success 'invalid unset' '
 	test_must_fail git config ${mode_unset} somesection.nonewline
 '
 
-cat > expect << EOF
+test_expect_success 'multivar unset' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
 [nextSection]
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar unset' '
 	case "$mode" in
 	legacy)
 		git config --unset nextsection.nonewline "wow3$";;
@@ -466,9 +463,10 @@ test_expect_success 'hierarchical section' '
 	git config Version.1.2.3eX.Alpha beta
 '
 
-cat > expect << EOF
+test_expect_success 'hierarchical section value' '
+	cat >expect <<EOF &&
 [beta] ; silly comment # another comment
-noIndent= sillyValue ; 'nother silly comment
+noIndent= sillyValue ; ${SQ}nother silly comment
 
 # empty line
 		; comment
@@ -479,19 +477,16 @@ noIndent= sillyValue ; 'nother silly comment
 [Version "1.2.3eX"]
 	Alpha = beta
 EOF
-
-test_expect_success 'hierarchical section value' '
 	test_cmp expect .git/config
 '
 
-cat > 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 <<EOF
-$HOME/
-/dev/null
-foo~
-EOF
-
 test_expect_success HOMEVAR 'get --path' '
+	cat >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
 '

-- 
2.51.0.536.g15c5d4f767.dirty

[PATCH v4 2/5] t1300: small style fixups

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 538f2c9b8a..6d1015acfd 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.

-- 
2.51.0.536.g15c5d4f767.dirty

[PATCH v4 3/5] builtin/config: do not die in `get_color()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 59fb113b07..afd48bfa51 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);

-- 
2.51.0.536.g15c5d4f767.dirty

[PATCH v4 4/5] builtin/config: special-case retrieving colors without a key

[Patrick Steinhardt]

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 has provided a default value,

then we call `get_color()` directly. Do so to make the documented
command work as expected.

[1]: <aI+oQvQgnNtC6DVw@szeder.dev>

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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 afd48bfa51..f50c11df57 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 6d1015acfd..3cf5d17aba 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.raw >actual &&
 	echo "<RED>" >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.raw >actual &&
+	echo "<RED>" >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]

-- 
2.51.0.536.g15c5d4f767.dirty

[PATCH v4 5/5] builtin/config: do not spawn pager when printing color codes

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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 f50c11df57..6708d91814 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 3cf5d17aba..358d636379 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.raw >actual &&
+	echo "<RED>" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'set --type=color' '
 	cat >expect <<\EOF &&
 [foo]

-- 
2.51.0.536.g15c5d4f767.dirty