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

Re: Two bugs in --pretty with %C(auto)

From
René Scharfe <l.s.r@web.de>
Date
Sep 29, 2016, 18:13 UTC
Message-ID
<b032fcbc-045b-7822-f9d4-84930cbf6b41@web.de>
In-Reply-To
<db20ae0c-9c33-1e65-b201-1b6a9ed11340@web.de>
Am 17.09.2016 um 20:25 schrieb René Scharfe:
Show 13 quoted lines
> diff --git a/pretty.c b/pretty.c
> index 9788bd8..493edb0 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1072,6 +1072,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  	case 'C':
>  		if (starts_with(placeholder + 1, "(auto)")) {
>  			c->auto_color = want_color(c->pretty_ctx->color);
> +			if (c->auto_color)
> +				strbuf_addstr(sb, GIT_COLOR_RESET);
>  			return 7; /* consumed 7 bytes, "C(auto)" */
>  		} else {
>  			int ret = parse_color(sb, placeholder, c);

We could optimize this a bit (see below). I can't think of a downside; someone adding a prefix would be responsible for adding a reset as well if needed, right?

-- >8 --
Subject: [PATCH] pretty: avoid adding reset for %C(auto) if output is empty

We emit an escape sequence for resetting color and attribute for %C(auto) to make sure automatic coloring is displayed as intended. Stop doing that if the output strbuf is empty, i.e. when %C(auto) appears at the start of the format string, because then there is no need for a reset and we save a few bytes in the output.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Reverts the change to t6006, so we'd need another test for this.
Anatoly? :)
 pretty.c                   | 2 +-
 t/t6006-rev-list-format.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/pretty.c b/pretty.c
index 493edb0..25efbca 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1072,7 +1072,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	case 'C':
 		if (starts_with(placeholder + 1, "(auto)")) {
 			c->auto_color = want_color(c->pretty_ctx->color);
-			if (c->auto_color)
+			if (c->auto_color && sb->len)
 				strbuf_addstr(sb, GIT_COLOR_RESET);
 			return 7; /* consumed 7 bytes, "C(auto)" */
 		} else {
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f6020cd..a1dcdb8 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -225,7 +225,7 @@ test_expect_success '%C(auto,...) respects --color=auto (stdout not tty)' '
 
 test_expect_success '%C(auto) respects --color' '
 	git log --color --format="%C(auto)%H" -1 >actual &&
-	printf "\\033[m\\033[33m%s\\033[m\\n" $(git rev-parse HEAD) >expect &&
+	printf "\\033[33m%s\\033[m\\n" $(git rev-parse HEAD) >expect &&
 	test_cmp expect actual
 '
 
-- 
2.10.0
Previous: Duy NguyenNext: Anatoly Borodin
Message 6 of 7 in “Two bugs in --pretty with %C(auto)”
  1. Anatoly BorodinSep 17, 2016
  2. René ScharfeSep 17, 2016
  3. Anatoly BorodinSep 18, 2016
  4. René ScharfeSep 18, 2016
  5. Duy NguyenSep 19, 2016
  6. René ScharfeSep 29, 2016
  7. Anatoly BorodinOct 1, 2016

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.