git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] builtin/config.c: don't print a newline with --color

From
Taylor Blau <me@ttaylorr.com>
Date
Mar 2, 2019, 00:40 UTC
Message-ID
<b5ca6391fd0273fb7d6b92bc5ada96df93bc5cf2.1551487219.git.me@ttaylorr.com>

Invocations of 'git config <name>' print a trailing newline after writing the value assigned to the given configuration variable.

This has an unexpected interaction with 63e2a0f8e9 (builtin/config: introduce `color` type specifier, 2018-04-09), which causes a newline to be printed after a color's ANSI escape sequence.

In this case, printing a terminating newline is not desirable. Callers may want to print out such a configuration variable in a sub-shell in order to color something else, in which case they certainly don't want a newline.

This bug has survived because there was never a test that would have caught it. The old test used 'test_decode_color', which checks that its input begins with a color, but stops parsing once it has parsed a single color successfully. In this case, it was ignoring the trailing '\n'.

To do what callers expect, only print a newline when the type is not 'color', and print the escape sequence itself for an exact comparison.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/config.c  | 3 ++-
 t/t1300-config.sh | 5 ++---
 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 98d65bc0ad..c8f088af38 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -258,7 +258,8 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
 				strbuf_setlen(buf, buf->len - 1);
 		}
 	}
-	strbuf_addch(buf, term);
+	if (type != TYPE_COLOR)
+		strbuf_addch(buf, term);
 	return 0;
 }
 
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 428177c390..ec1b3a852d 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -907,9 +907,8 @@ test_expect_success 'get --expiry-date' '
 test_expect_success 'get --type=color' '
 	rm .git/config &&
 	git config foo.color "red" &&
-	git config --get --type=color foo.color >actual.raw &&
-	test_decode_color <actual.raw >actual &&
-	echo "<RED>" >expect &&
+	printf "\\033[31m" >expect &&
+	git config --get --type=color foo.color >actual &&
 	test_cmp expect actual
 '
 
-- 
2.20.1
Next: Eric Sunshine
Message 1 of 18 in “builtin/config.c: don't print a newline with --color”
  1. builtin/config.c: don't print a newline with --colorTaylor Blau, Mar 2, 2019
  2. Eric SunshineMar 2, 2019
  3. Johannes SchindelinMar 2, 2019
  4. Jeff KingMar 3, 2019
  5. Johannes SchindelinMar 5, 2019
  6. Taylor BlauMar 6, 2019
  7. Junio C HamanoMar 3, 2019
  8. Jeff KingMar 3, 2019
  9. Junio C HamanoMar 4, 2019
  10. Junio C HamanoMar 4, 2019
  11. Jeff KingMar 5, 2019
  12. Junio C HamanoMar 5, 2019
  13. Taylor BlauMar 6, 2019
  14. Junio C HamanoMar 7, 2019
  15. Jeff KingMar 7, 2019
  16. Documentation/config: note trailing newline with --type=colorTaylor Blau, Mar 6, 2019
  17. Jeff KingMar 7, 2019
  18. Junio C HamanoMar 7, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.