{"thread":{"id":"42464","subject":"[PATCH] format_commit_message: honor `color=auto` for `%C(auto)`","startedAt":"2016-05-27T03:46:10Z","lastAt":"2016-05-27T06:22:36Z","messageCount":3,"participants":["Edward Thomson","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287643","messageId":"20160527034610.GA31629@zoidberg","threadId":"42464","inReplyTo":null,"subject":"[PATCH] format_commit_message: honor `color=auto` for `%C(auto)`","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2016-05-27T03:46:10Z","receivedAt":"2016-05-27T03:46:10Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"git-log(1) documents that when specifying the `%C(auto)` format\nplaceholder will \"turn on auto coloring on the next %placeholders\nuntil the color is switched again.\"\n\nHowever, when `%C(auto)` is used, the present implementation will turn\ncolors on unconditionally (even if the color configuration is turned off\nfor the current context - for example, `--no-color` was specified or the\ncolor is `auto` and the output is not a tty).\n\nUpdate `format_commit_one` to examine the current context when a format\nstring of `%C(auto)` is specified, which ensures that we will not\nunconditionally write colors.  This brings that behavior in line with\nthe behavior of `%C(auto,<colorname>)`, and allows the user the ability\nto specify that color should be displayed only when the output is a\ntty.\n\nAdditionally, add a test for `%C(auto)` and update the existing tests\nfor `%C(auto,...)` as they were misidentified as being applicable to\n`%C(auto)`.\n\nSigned-off-by: Edward Thomson <ethomson@edwardthomson.com>\n\nTests from Jeff King.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pretty.c                   |  2 +-\n t/t6006-rev-list-format.sh | 26 +++++++++++++++++++-------\n 2 files changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 87c4497..c3ec430 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1063,7 +1063,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \tswitch (placeholder[0]) {\n \tcase 'C':\n \t\tif (starts_with(placeholder + 1, \"(auto)\")) {\n-\t\t\tc->auto_color = 1;\n+\t\t\tc->auto_color = want_color(c->pretty_ctx->color);\n \t\t\treturn 7; /* consumed 7 bytes, \"C(auto)\" */\n \t\t} else {\n \t\t\tint ret = parse_color(sb, placeholder, c);\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex b77d4c9..a1dcdb8 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -184,38 +184,38 @@ commit $head1\n \u001b[1;31;43mfoo\u001b[m\n EOF\n \n-test_expect_success '%C(auto) does not enable color by default' '\n+test_expect_success '%C(auto,...) does not enable color by default' '\n \tgit log --format=$AUTO_COLOR -1 >actual &&\n \thas_no_color actual\n '\n \n-test_expect_success '%C(auto) enables colors for color.diff' '\n+test_expect_success '%C(auto,...) enables colors for color.diff' '\n \tgit -c color.diff=always log --format=$AUTO_COLOR -1 >actual &&\n \thas_color actual\n '\n \n-test_expect_success '%C(auto) enables colors for color.ui' '\n+test_expect_success '%C(auto,...) enables colors for color.ui' '\n \tgit -c color.ui=always log --format=$AUTO_COLOR -1 >actual &&\n \thas_color actual\n '\n \n-test_expect_success '%C(auto) respects --color' '\n+test_expect_success '%C(auto,...) respects --color' '\n \tgit log --format=$AUTO_COLOR -1 --color >actual &&\n \thas_color actual\n '\n \n-test_expect_success '%C(auto) respects --no-color' '\n+test_expect_success '%C(auto,...) respects --no-color' '\n \tgit -c color.ui=always log --format=$AUTO_COLOR -1 --no-color >actual &&\n \thas_no_color actual\n '\n \n-test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '\n+test_expect_success TTY '%C(auto,...) respects --color=auto (stdout is tty)' '\n \ttest_terminal env TERM=vt100 \\\n \t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n \thas_color actual\n '\n \n-test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '\n+test_expect_success '%C(auto,...) respects --color=auto (stdout not tty)' '\n \t(\n \t\tTERM=vt100 && export TERM &&\n \t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n@@ -223,6 +223,18 @@ test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '\n \t)\n '\n \n+test_expect_success '%C(auto) respects --color' '\n+\tgit log --color --format=\"%C(auto)%H\" -1 >actual &&\n+\tprintf \"\\\\033[33m%s\\\\033[m\\\\n\" $(git rev-parse HEAD) >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '%C(auto) respects --no-color' '\n+\tgit log --no-color --format=\"%C(auto)%H\" -1 >actual &&\n+\tgit rev-parse HEAD >expect &&\n+\ttest_cmp expect actual\n+'\n+\n iconv -f utf-8 -t $test_encoding > commit-msg <<EOF\n Test printing of complex bodies\n \n-- \n2.7.4 (Apple Git-66)\n"},{"id":"287645","messageId":"20160527035553.GA24972@sigill.intra.peff.net","threadId":"42464","inReplyTo":"20160527034610.GA31629@zoidberg","subject":"Re: [PATCH] format_commit_message: honor `color=auto` for `%C(auto)`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-27T03:55:54Z","receivedAt":"2016-05-27T03:55:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 26, 2016 at 10:46:10PM -0500, Edward Thomson wrote:\n\n> git-log(1) documents that when specifying the `%C(auto)` format\n> placeholder will \"turn on auto coloring on the next %placeholders\n> until the color is switched again.\"\n> \n> However, when `%C(auto)` is used, the present implementation will turn\n> colors on unconditionally (even if the color configuration is turned off\n> for the current context - for example, `--no-color` was specified or the\n> color is `auto` and the output is not a tty).\n> \n> Update `format_commit_one` to examine the current context when a format\n> string of `%C(auto)` is specified, which ensures that we will not\n> unconditionally write colors.  This brings that behavior in line with\n> the behavior of `%C(auto,<colorname>)`, and allows the user the ability\n> to specify that color should be displayed only when the output is a\n> tty.\n> \n> Additionally, add a test for `%C(auto)` and update the existing tests\n> for `%C(auto,...)` as they were misidentified as being applicable to\n> `%C(auto)`.\n\nExplanation and the patch look good.\n\n> Signed-off-by: Edward Thomson <ethomson@edwardthomson.com>\n> \n> Tests from Jeff King.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n\nTrailers should all go at the bottom in a single stanza, and should\ngenerally be in chronological order (so you got the bits from with an\ns-o-b, and then you signed off the whole thing). IOW:\n\n> Tests from Jeff King.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> Signed-off-by: Edward Thomson <ethomson@edwardthomson.com>\n\nI suspect Junio can just tweak that while applying, unless there's\nanother reason to re-roll.\n\n(Also for anybody watching, Ed did not just make up my signoff; I gave\nit to him off-list).\n\n-Peff\n"},{"id":"287657","messageId":"xmqqwpmgrpeb.fsf@gitster.mtv.corp.google.com","threadId":"42464","inReplyTo":"20160527035553.GA24972@sigill.intra.peff.net","subject":"Re: [PATCH] format_commit_message: honor `color=auto` for `%C(auto)`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-27T06:22:36Z","receivedAt":"2016-05-27T06:22:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I suspect Junio can just tweak that while applying, unless there's\n> another reason to re-roll.\n>\n> (Also for anybody watching, Ed did not just make up my signoff; I gave\n> it to him off-list).\n\nUnderstood.  Thanks.\n"}]}