threads / patch / 64128

patch, 5 partsbuiltin/config: bug fixes for "get" subcommand with "--type=color"

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

## tl;dr

36 messages between Sep 11, 2025 and Sep 22, 2025. Diffs are folded; open one to read it.

replies: 35people: 3as markdown or json

Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · lore
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

Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · re: Patrick Steinhardt · lore

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

There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies.

Convert those to instead do so as part of the test itself.

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(-)
Show changes to t/t1300-config.sh +129 −161
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
Kristoffer Haugsbakk· Sep 11, 2025, 16:50 UTC · re: Patrick Steinhardt · lore

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

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

Show 13 quoted lines
> 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.
Show 12 quoted lines
>  	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.
Show 14 quoted lines
>  	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.
Show 15 quoted lines
>  	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.
Show 20 quoted lines
>  	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.
Show 13 quoted lines
>  	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`.

Show 25 quoted lines
>
>  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 `\`.
Show 22 quoted lines
>
>  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.

Show 21 quoted lines
>
> @@ -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.
Show 15 quoted lines
>
> -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.
Show 21 quoted lines
>  	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.

Show 20 quoted lines
> @@ -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.
Show 21 quoted lines
>
> -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.
Show 18 quoted lines
>
> -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.
Show 21 quoted lines
> @@ -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.
Show 21 quoted lines
> @@ -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.
Show 18 quoted lines
>
> -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.
Show 22 quoted lines
> @@ -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`.
Show 14 quoted lines
>
> -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.
Show 14 quoted lines
>
> -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.
Show 14 quoted lines
>
> -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.
Show 11 quoted lines
> @@ -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.
Show 8 quoted lines
>
> -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.
Show 8 quoted lines
>
> -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.
Show 8 quoted lines
>
> -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.
Show 8 quoted lines
>
> -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`.
Show 17 quoted lines
> @@ -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`.
Show 16 quoted lines
>
> -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`.
Show 12 quoted lines
> @@ -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.
Show 20 quoted lines
>  	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`.
Show 18 quoted lines
> @@ -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.)
Show 20 quoted lines
>
> @@ -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.
Show 17 quoted lines
>
> @@ -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.
Show 19 quoted lines
>
> -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.
Show 32 quoted lines
>
>  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.
Show 20 quoted lines
> @@ -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)
Show 18 quoted lines
> @@ -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.
Show 19 quoted lines
> @@ -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. :)
Show 17 quoted lines
> @@ -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.
Show 21 quoted lines
> @@ -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.
Show 14 quoted lines
>
> -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.
Show 15 quoted lines
> @@ -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`.
Show 18 quoted lines
> @@ -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`.
Show 16 quoted lines
> @@ -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
Patrick Steinhardt· Sep 15, 2025, 11:41 UTC · re: Kristoffer Haugsbakk · lore

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

On Thu, Sep 11, 2025 at 06:50:29PM +0200, Kristoffer Haugsbakk wrote:
Show 17 quoted lines
> 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.
Show 23 quoted lines
> > 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.

Show 17 quoted lines
> > @@ -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
Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · re: Patrick Steinhardt · lore

[PATCH 2/5] t1300: small style fixups

We have a couple of small style violations in t1300:
  - An empty newline at the start of the test body.
  - The test command is sometimes on the same line as the test name.
  - The closing single-quote is sometimes on the same line as the last
    command of the test.
Fix these.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)
Show changes to t/t1300-config.sh +21 −13
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
Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · re: Patrick Steinhardt · lore

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

When trying to parse an invalid color via `get_color()` we die. We're about to introduce another caller in a subsequent commit though that has its own error handling, so dying is a bit drastic there. Furthermore, the only caller that we already have right now already knows to handle errors in other branches that don't call `get_color()`.

Convert the function to instead return an error code to improve its flexibility.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
Show changes to builtin/config.c +11 −4
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
Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 2 files +13 −0

builtin/config.c, t/t1300-config.sh

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
Kristoffer Haugsbakk· Sep 11, 2025, 16:48 UTC · re: Patrick Steinhardt · lore

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

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.
Show 7 quoted lines
> 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.
Show 9 quoted lines
> 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.
Show 5 quoted lines
>
> 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.

Show 6 quoted lines
>
> [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]
Patrick Steinhardt· Sep 15, 2025, 11:41 UTC · re: Kristoffer Haugsbakk · lore

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

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
Show 9 quoted lines
> > 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.
Show 21 quoted lines
> > 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
Patrick Steinhardt· Sep 11, 2025, 13:24 UTC · re: Patrick Steinhardt · lore

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

With `git config get --type=color` the user asks us to parse a specific configuration key and turn the value into an ANSI color escape sequence. The printed string can then for example be used as part of shell scripts to reuse the same colors as Git.

Right now though we set up the auto-pager 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(-)
Show changes to 2 files +11 −1

builtin/config.c, t/t1300-config.sh

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
Kristoffer Haugsbakk· Sep 11, 2025, 16:49 UTC · re: Patrick Steinhardt · lore

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

On Thu, Sep 11, 2025, at 15:24, Patrick Steinhardt wrote:
Show 6 quoted lines
> 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/
Show 7 quoted lines
> 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]
Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

Hi,

this small patch series contains two bug fixes for `git config get --type=color`:

  - We restore the behaviour where we can now parse colors without a
    config key.
  - We stop spawning the pager when the user requests to print ANSI
    color escape sequences.

Furthermore, the patch series does some lighter refactorings of t1300. That test file still has its fair share of issues, but at least it looks a bit less dirty now.

Changes in v2:
  - Improve commit messages.
  - Use "\EOF" and "-EOF" in more cases.
  - Move a style fixup from the first commit into the second commit.
  - Link to v1: https://lore.kernel.org/r/20250911-pks-config-color-v1-0-3a7c79df65b1@pks.im
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

Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies.

Convert those to instead do so as part of the test itself. While at it, normalize these tests to use `<<\EOF` for those that don't use variable expansion and `<<-EOF` for those that aren't sensitive to indentation.

Note that there are two exceptions that we leave as-is for now since they are reused across tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 291 ++++++++++++++++++++++++------------------------------
 1 file changed, 128 insertions(+), 163 deletions(-)
Show changes to t/t1300-config.sh +128 −163
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
Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

We have a couple of small style violations in t1300:
  - An empty newline at the start of the test body.
  - The test command is sometimes on the same line as the test name.
  - The closing single-quote is sometimes on the same line as the last
    command of the test.
Fix these.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)
Show changes to t/t1300-config.sh +24 −14
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
Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

When trying to parse an invalid color via `get_color()` we die. We're about to introduce another caller in a subsequent commit though that has its own error handling, so dying is a bit drastic there. Furthermore, the only caller that we already have right now already knows to handle errors in other branches that don't call `get_color()`.

Convert the function to instead return an error code to improve its flexibility.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
Show changes to builtin/config.c +11 −4
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
Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 2 files +13 −0

builtin/config.c, t/t1300-config.sh

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
Junio C Hamano· Sep 15, 2025, 17:19 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 37 quoted lines
> 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.
Patrick Steinhardt· Sep 15, 2025, 12:52 UTC · re: Patrick Steinhardt · lore

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

With `git config get --type=color` the user asks us to parse a specific configuration key and turn the value into an ANSI color escape sequence. The printed string can then for example be used as part of shell scripts to reuse the same colors as Git.

Right now though we set up the auto-pager, which means that the string may 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(-)
Show changes to 2 files +11 −1

builtin/config.c, t/t1300-config.sh

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
Junio C Hamano· Sep 15, 2025, 17:28 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 12 quoted lines
> 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...

Kristoffer Haugsbakk· Sep 16, 2025, 06:56 UTC · re: Junio C Hamano · lore

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

On Mon, Sep 15, 2025, at 19:28, Junio C Hamano wrote:
Show 7 quoted lines
> 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?
Show 7 quoted lines
>
> An interactive user experimenting while writing their own script
> might say something like
>
>  $ git config --type=color --default="reverse red" n.n
>
>[snip]
Patrick Steinhardt· Sep 18, 2025, 06:03 UTC · re: Junio C Hamano · lore

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

On Mon, Sep 15, 2025 at 10:28:23AM -0700, Junio C Hamano wrote:
Show 33 quoted lines
> 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
Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

Hi,

this small patch series contains two bug fixes for `git config get --type=color`:

  - We restore the behaviour where we can now parse colors without a
    config key.
  - We stop spawning the pager when the user requests to print ANSI
    color escape sequences.

Furthermore, the patch series does some lighter refactorings of t1300. That test file still has its fair share of issues, but at least it looks a bit less dirty now.

Changes in v2:
  - Improve commit messages.
  - Use "\EOF" and "-EOF" in more cases.
  - Move a style fixup from the first commit into the second commit.
  - Link to v1: https://lore.kernel.org/r/20250911-pks-config-color-v1-0-3a7c79df65b1@pks.im
Changes in v3:
  - Provide additional context as part of the commit message for the
    commit that stops setting up the pager with `--type=color`.
  - Link to v2: https://lore.kernel.org/r/20250915-pks-config-color-v2-0-e4290bd8d13c@pks.im
Thanks!
Patrick
---
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

Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies.

Convert those to instead do so as part of the test itself. While at it, normalize these tests to use `<<\EOF` for those that don't use variable expansion and `<<-EOF` for those that aren't sensitive to indentation.

Note that there are two exceptions that we leave as-is for now since they are reused across tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 291 ++++++++++++++++++++++++------------------------------
 1 file changed, 128 insertions(+), 163 deletions(-)
Show changes to t/t1300-config.sh +128 −163
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
Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

We have a couple of small style violations in t1300:
  - An empty newline at the start of the test body.
  - The test command is sometimes on the same line as the test name.
  - The closing single-quote is sometimes on the same line as the last
    command of the test.
Fix these.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)
Show changes to t/t1300-config.sh +24 −14
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
Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

When trying to parse an invalid color via `get_color()` we die. We're about to introduce another caller in a subsequent commit though that has its own error handling, so dying is a bit drastic there. Furthermore, the only caller that we already have right now already knows to handle errors in other branches that don't call `get_color()`.

Convert the function to instead return an error code to improve its flexibility.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
Show changes to builtin/config.c +11 −4
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
Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 2 files +13 −0

builtin/config.c, t/t1300-config.sh

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
Junio C Hamano· Sep 18, 2025, 06:49 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 32 quoted lines
> 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?
Show 5 quoted lines
>
>     - 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 ;-)

Patrick Steinhardt· Sep 22, 2025, 13:04 UTC · re: Junio C Hamano · lore

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

On Wed, Sep 17, 2025 at 11:49:18PM -0700, Junio C Hamano wrote:
Show 36 quoted lines
> 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.
Show 16 quoted lines
> >     - 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
Junio C Hamano· Sep 22, 2025, 16:29 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 15 quoted lines
>> 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.
Patrick Steinhardt· Sep 18, 2025, 06:14 UTC · re: Patrick Steinhardt · lore

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

With `git config get --type=color` the user asks us to parse a specific configuration key and turn the value into an ANSI color escape sequence. The printed string can then for example be used as part of shell scripts to reuse the same colors as Git.

Right now though we set up the auto-pager, which means that the string may be written to the pager instead of directly to the terminal. This behaviour is problematic for two reasons:

  - Color codes are meant for direct terminal output; writing them into
    a pager does not seem like a sensible thing to do without additional
    text.
  - It is inconsistent with `git config --get-color`, which never uses a
    pager, despite the fact that we claim `git config get --type=color`
    to be a drop-in replacement in git-config(1).
Fix this by disabling the pager when outputting color sequences.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c  | 3 ++-
 t/t1300-config.sh | 9 +++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
Show changes to 2 files +11 −1

builtin/config.c, t/t1300-config.sh

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
Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

Hi,

this small patch series contains two bug fixes for `git config get --type=color`:

  - We restore the behaviour where we can now parse colors without a
    config key.
  - We stop spawning the pager when the user requests to print ANSI
    color escape sequences.

Furthermore, the patch series does some lighter refactorings of t1300. That test file still has its fair share of issues, but at least it looks a bit less dirty now.

Changes in v2:
  - Improve commit messages.
  - Use "\EOF" and "-EOF" in more cases.
  - Move a style fixup from the first commit into the second commit.
  - Link to v1: https://lore.kernel.org/r/20250911-pks-config-color-v1-0-3a7c79df65b1@pks.im
Changes in v3:
  - Provide additional context as part of the commit message for the
    commit that stops setting up the pager with `--type=color`.
  - Link to v2: https://lore.kernel.org/r/20250915-pks-config-color-v2-0-e4290bd8d13c@pks.im
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

Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

There are a bunch of tests in t1300 where we write the test expectation handed over to `test_cmp ()` outside of the test body. This does not match our modern test style, and there isn't really a reason why this would need to happen outside of the test bodies.

Convert those to instead do so as part of the test itself. While at it, normalize these tests to use `<<\EOF` for those that don't use variable expansion and `<<-EOF` for those that aren't sensitive to indentation.

Note that there are two exceptions that we leave as-is for now since they are reused across tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 291 ++++++++++++++++++++++++------------------------------
 1 file changed, 128 insertions(+), 163 deletions(-)
Show changes to t/t1300-config.sh +128 −163
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
Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

We have a couple of small style violations in t1300:
  - An empty newline at the start of the test body.
  - The test command is sometimes on the same line as the test name.
  - The closing single-quote is sometimes on the same line as the last
    command of the test.
Fix these.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1300-config.sh | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)
Show changes to t/t1300-config.sh +24 −14
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
Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

When trying to parse an invalid color via `get_color()` we die. We're about to introduce another caller in a subsequent commit though that has its own error handling, so dying is a bit drastic there. Furthermore, the only caller that we already have right now already knows to handle errors in other branches that don't call `get_color()`.

Convert the function to instead return an error code to improve its flexibility.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
Show changes to builtin/config.c +11 −4
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
Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 2 files +13 −0

builtin/config.c, t/t1300-config.sh

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
Patrick Steinhardt· Sep 22, 2025, 13:06 UTC · re: Patrick Steinhardt · lore

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

With `git config get --type=color` the user asks us to parse a specific configuration key and turn the value into an ANSI color escape sequence. The printed string can then for example be used as part of shell scripts to reuse the same colors as Git.

Right now though we set up the auto-pager, which means that the string may be written to the pager instead of directly to the terminal. This behaviour is problematic for two reasons:

  - Color codes are meant for direct terminal output; writing them into
    a pager does not seem like a sensible thing to do without additional
    text.
  - It is inconsistent with `git config --get-color`, which never uses a
    pager, despite the fact that we claim `git config get --type=color`
    to be a drop-in replacement in git-config(1).
Fix this by disabling the pager when outputting color sequences.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/config.c  | 3 ++-
 t/t1300-config.sh | 9 +++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
Show changes to 2 files +11 −1

builtin/config.c, t/t1300-config.sh

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

← back to recent threads