{"thread":{"id":"32324","subject":"FW: Git log --graph doesn't output color when redirected","startedAt":"2012-12-12T17:35:17Z","lastAt":"2012-12-17T22:59:58Z","messageCount":18,"participants":["Srb, Michal","Jeff King","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"204759","messageId":"72BB37CB88C48F4B925365539F1EE46C18261403@icexch-m3.ic.ac.uk","threadId":"32324","inReplyTo":"72BB37CB88C48F4B925365539F1EE46C182613A9@icexch-m3.ic.ac.uk","subject":"FW: Git log --graph doesn't output color when redirected","fromName":"Srb, Michal","fromEmail":"michal.srb11@imperial.ac.uk","sentAt":"2012-12-12T17:35:17Z","receivedAt":"2012-12-12T17:35:17Z","isPatch":false,"sender":{"key":"michal.srb11@imperial.ac.uk","avatar":null},"body":"Unlike --pretty-format, --graph doesn’t output colors when the git log output\nis redirected.\n \nTested on Ubuntu 12.04 and msys on Windows 8.\n \nIs there a setting somewhere in config to change this?\n \nThanks,\n \nMichal\n"},{"id":"204832","messageId":"20121213131329.GA5042@sigill.intra.peff.net","threadId":"32324","inReplyTo":"72BB37CB88C48F4B925365539F1EE46C18261403@icexch-m3.ic.ac.uk","subject":"Re: FW: Git log --graph doesn't output color when redirected","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-13T13:13:29Z","receivedAt":"2012-12-13T13:13:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 12, 2012 at 05:35:17PM +0000, Srb, Michal wrote:\n\n> Unlike --pretty-format, --graph doesn’t output colors when the git log output\n> is redirected.\n\nI do not think it has anything to do with --graph in particular, but\nrather that when colorization is set to the \"auto\" mode, it is enabled\nonly when stdout is a tty. Diff coloring, for example, follows the same\nrules. If you are using --format=\"%C(red)\" or similar placeholders,\nthey are the odd duck by not respecting the auto-color mode.\n\n> Is there a setting somewhere in config to change this?\n\nYes. If you use \"--color\" on the command line, that means\n\"unconditionally use color\". If you set color.ui (or any other color\nconfig option) to \"always\", then you will always get color (and you can\ndisable it for a particular run with \"--no-color\"). Setting a color\nconfig option to \"true\" is the same as \"auto\", which gets you the auto\nmode.\n\n-Peff\n"},{"id":"204835","messageId":"72BB37CB88C48F4B925365539F1EE46C1826168A@icexch-m3.ic.ac.uk","threadId":"32324","inReplyTo":"20121213131329.GA5042@sigill.intra.peff.net","subject":"RE: FW: Git log --graph doesn't output color when redirected","fromName":"Srb, Michal","fromEmail":"michal.srb11@imperial.ac.uk","sentAt":"2012-12-13T15:34:29Z","receivedAt":"2012-12-13T15:34:29Z","isPatch":false,"sender":{"key":"michal.srb11@imperial.ac.uk","avatar":null},"body":"From: Jeff King [peff@peff.net]\nSent: Thursday, December 13, 2012 1:13 PM\n\n>> Is there a setting somewhere in config to change this?\n\n> Yes. If you use \"--color\" on the command line, that means\n> \"unconditionally use color\". If you set color.ui (or any other\n> color config option) to \"always\", then you will always get color (and\n> you can disable it for a particular run with \"--no-color\"). Setting a color\n> config option to \"true\" is the same as \"auto\", which gets you \n> the auto mode.\n \nSetting color in gitconfig didn't work for me on msys, but --color \nworks like magic, thanks!"},{"id":"204894","messageId":"CACsJy8AgtwtJfMXzArJLiHQtR+WNRJxRdRgUts30EN-QvgGT=g@mail.gmail.com","threadId":"32324","inReplyTo":"20121213131329.GA5042@sigill.intra.peff.net","subject":"Re: FW: Git log --graph doesn't output color when redirected","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-12-15T03:23:10Z","receivedAt":"2012-12-15T03:23:10Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 13, 2012 at 8:13 PM, Jeff King wrote:\n> If you are using --format=\"%C(red)\" or similar placeholders,\n> they are the odd duck by not respecting the auto-color mode.\n\nBut they should, shouldn't they? Just asking. I may do it to when I\nrevive nd/pretty-placeholder-with-color-option.\n-- \nDuy\n"},{"id":"204903","messageId":"20121215101659.GB21577@sigill.intra.peff.net","threadId":"32324","inReplyTo":"CACsJy8AgtwtJfMXzArJLiHQtR+WNRJxRdRgUts30EN-QvgGT=g@mail.gmail.com","subject":"Re: FW: Git log --graph doesn't output color when redirected","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-15T10:16:59Z","receivedAt":"2012-12-15T10:16:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 15, 2012 at 10:23:10AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> On Thu, Dec 13, 2012 at 8:13 PM, Jeff King wrote:\n> > If you are using --format=\"%C(red)\" or similar placeholders,\n> > they are the odd duck by not respecting the auto-color mode.\n> \n> But they should, shouldn't they? Just asking. I may do it to when I\n> revive nd/pretty-placeholder-with-color-option.\n\nIf I were designing --format today, I would certainly say so. The only\nthing holding me back would be backwards compatibility. We could get\naround that by introducing a new placeholder like %c(color) that behaves\nlike %C(color), except respects the --color flag.\n\nIt looks like this came up before:\n\n http://article.gmane.org/gmane.comp.version-control.git/169225\n\nbut never got pushed to inclusion.\n\n-Peff\n"},{"id":"204930","messageId":"7v7gojtbgr.fsf@alter.siamese.dyndns.org","threadId":"32324","inReplyTo":"20121215101659.GB21577@sigill.intra.peff.net","subject":"Re: FW: Git log --graph doesn't output color when redirected","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-15T18:30:12Z","receivedAt":"2012-12-15T18:30:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Dec 15, 2012 at 10:23:10AM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>> On Thu, Dec 13, 2012 at 8:13 PM, Jeff King wrote:\n>> > If you are using --format=\"%C(red)\" or similar placeholders,\n>> > they are the odd duck by not respecting the auto-color mode.\n>> \n>> But they should, shouldn't they? Just asking. I may do it to when I\n>> revive nd/pretty-placeholder-with-color-option.\n>\n> If I were designing --format today, I would certainly say so. The only\n> thing holding me back would be backwards compatibility. We could get\n> around that by introducing a new placeholder like %c(color) that behaves\n> like %C(color), except respects the --color flag.\n\nI think the %c(color) thing is a good way to go if we want to pursue\nthis.\n\nAnother possibility without wasting one more special letter would be\nto allow %C(auto,red), perhaps like this (untested):\n\n pretty.c | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git i/pretty.c w/pretty.c\nindex dba6828..77cf826 100644\n--- i/pretty.c\n+++ w/pretty.c\n@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tswitch (placeholder[0]) {\n \tcase 'C':\n \t\tif (placeholder[1] == '(') {\n-\t\t\tconst char *end = strchr(placeholder + 2, ')');\n+\t\t\tconst char *begin = placeholder + 2;\n+\t\t\tconst char *end = strchr(begin, ')');\n \t\t\tchar color[COLOR_MAXLEN];\n+\n \t\t\tif (!end)\n \t\t\t\treturn 0;\n-\t\t\tcolor_parse_mem(placeholder + 2,\n-\t\t\t\t\tend - (placeholder + 2),\n+\t\t\tif (!memcmp(begin, \"auto,\", 5)) {\n+\t\t\t\tif (!want_color(GIT_COLOR_AUTO))\n+\t\t\t\t\treturn 0;\n+\t\t\t\tbegin += 5;\n+\t\t\t}\n+\t\t\tcolor_parse_mem(begin,\n+\t\t\t\t\tend - begin,\n \t\t\t\t\t\"--pretty format\", color);\n \t\t\tstrbuf_addstr(sb, color);\n \t\t\treturn end - placeholder + 1;\n"},{"id":"205032","messageId":"7vobhtm5pk.fsf_-_@alter.siamese.dyndns.org","threadId":"32324","inReplyTo":"7v7gojtbgr.fsf@alter.siamese.dyndns.org","subject":"[PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T08:40:55Z","receivedAt":"2012-12-17T08:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Traditionally, %C(color attr) always emitted the ANSI color\nsequence; it was up to the scripts that wanted to conditionally\ncolor their output to omit %C(...) specifier when they do not want\ncolors.\n\nOptionally allow \"auto,\" to be prefixed to the color, so that the\noutput is colored iff it goes to the terminal.\n\nSigned-off-by: Junio C Hamano \n---\n * This time with minimum test and documentation.\n\n Documentation/pretty-formats.txt | 4 +++-\n pretty.c | 13 ++++++++++---\n t/t6006-rev-list-format.sh | 30 ++++++++++++++++++++++++++----\n 3 files changed, 39 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex d9edded..2af017c 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -144,7 +144,9 @@ The placeholders are:\n - '%Cgreen': switch color to green\n - '%Cblue': switch color to blue\n - '%Creset': reset color\n-- '%C(...)': color specification, as described in color.branch.* config option\n+- '%C(...)': color specification, as described in color.branch.* config option;\n+ adding `auto,` at the beginning will emit color only when the\n+ output is going to a terminal\n - '%m': left, right or boundary mark\n - '%n': newline\n - '%%': a raw '%'\ndiff --git a/pretty.c b/pretty.c\nindex dba6828..b9fd972 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tswitch (placeholder[0]) {\n \tcase 'C':\n \t\tif (placeholder[1] == '(') {\n-\t\t\tconst char *end = strchr(placeholder + 2, ')');\n+\t\t\tconst char *begin = placeholder + 2;\n+\t\t\tconst char *end = strchr(begin, ')');\n \t\t\tchar color[COLOR_MAXLEN];\n+\n \t\t\tif (!end)\n \t\t\t\treturn 0;\n-\t\t\tcolor_parse_mem(placeholder + 2,\n-\t\t\t\t\tend - (placeholder + 2),\n+\t\t\tif (!memcmp(begin, \"auto,\", 5)) {\n+\t\t\t\tif (!want_color(-1))\n+\t\t\t\t\treturn end - placeholder + 1;\n+\t\t\t\tbegin += 5;\n+\t\t\t}\n+\t\t\tcolor_parse_mem(begin,\n+\t\t\t\t\tend - begin,\n \t\t\t\t\t\"--pretty format\", color);\n \t\t\tstrbuf_addstr(sb, color);\n \t\t\treturn end - placeholder + 1;\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex f94f0c4..bfcc1c6 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -11,12 +11,12 @@ touch foo && git add foo && git commit -m \"added foo\" &&\n '\n \n # usage: test_format name format_string expect.$1\n \ttest_expect_success \"format $1\" \"\n-git rev-list --pretty=format:'$2' master >output.$1 &&\n-test_cmp expect.$1 output.$1\n-\"\n+\t\tgit rev-list --pretty=format:'$2'${3:+ $3} master >output.$1 &&\n+\t\ttest_cmp expect.$1 output.$1\n+\t\"\n }\n \n test_format percent %%h <<'EOF'\n@@ -124,6 +124,28 @@ commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n \u001b[1;31;43mfoo\u001b[m\n EOF\n \n+test_format advanced-colors-auto \\\n+\t'%C(auto,red yellow bold)foo%C(auto,reset)' --color <<'EOF'\n+commit 131a310eb913d107dd3c09a65d1651175898735d\n+foo\n+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n+foo\n+EOF\n+\n+# %C(auto,...) should trump --color=always\n+#\n+# NEEDSWORK: --color=never should also be tested but we need to run a\n+# similar test under pseudo-terminal with test_terminal which is too\n+# much hassle for its worth.\n+\n+test_format advanced-colors-forced \\\n+\t'%C(auto,red yellow bold)foo%C(auto,reset)' --color=always <<'EOF'\n+commit 131a310eb913d107dd3c09a65d1651175898735d\n+foo\n+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n+foo\n+EOF\n+\n cat >commit-msg <<'EOF'\n Test printing of complex bodies\n \n-- \n1.8.1.rc2.126.ge9b7782\n"},{"id":"205050","messageId":"CACsJy8Dt4vEpO+EcAhWnko=XAajQ9OMgbDbVx78Eb=sZTjmKQA@mail.gmail.com","threadId":"32324","inReplyTo":"7vobhtm5pk.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-12-17T11:44:10Z","receivedAt":"2012-12-17T11:44:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 17, 2012 at 3:40 PM, Junio C Hamano wrote:\n> Traditionally, %C(color attr) always emitted the ANSI color\n> sequence; it was up to the scripts that wanted to conditionally\n> color their output to omit %C(...) specifier when they do not want\n> colors.\n>\n> Optionally allow \"auto,\" to be prefixed to the color, so that the\n> output is colored iff it goes to the terminal.\n\nI see \"prefix\" is clearly documented. Still it feels a bit unnatural\nthat %C(red,auto) won't work. But we can make that case work later if\nsomebody cares enough.\n\n> if (!end)\n> return 0;\n> - color_parse_mem(placeholder + 2,\n> - end - (placeholder + 2),\n> + if (!memcmp(begin, \"auto,\", 5)) {\n> + if (!want_color(-1))\n> + return end - placeholder + 1;\n\nThis want_color() checks color.ui and only when color.ui = auto, it\nbothers to check if the output is tty. I think the document should say\nthat \"auto,\" (or maybe another name because it's not really auto)\nrespects color.ui.\n-- \nDuy\n"},{"id":"205053","messageId":"20121217121354.GB21858@sigill.intra.peff.net","threadId":"32324","inReplyTo":"CACsJy8Dt4vEpO+EcAhWnko=XAajQ9OMgbDbVx78Eb=sZTjmKQA@mail.gmail.com","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T12:13:54Z","receivedAt":"2012-12-17T12:13:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2012 at 06:44:10PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> > if (!end)\n> > return 0;\n> > - color_parse_mem(placeholder + 2,\n> > - end - (placeholder + 2),\n> > + if (!memcmp(begin, \"auto,\", 5)) {\n> > + if (!want_color(-1))\n> > + return end - placeholder + 1;\n> \n> This want_color() checks color.ui and only when color.ui = auto, it\n> bothers to check if the output is tty. I think the document should say\n> that \"auto,\" (or maybe another name because it's not really auto)\n> respects color.ui.\n\nYeah, that should definitely be documented. I wonder if it should\nactually respect color.diff, which is what \"log\" usually uses (albeit\nmostly for the diff itself, we have always used it for the graph and for\nthe \"commit\" header of each entry).\n\n-Peff\n"},{"id":"205055","messageId":"20121217124029.GC21858@sigill.intra.peff.net","threadId":"32324","inReplyTo":"7vobhtm5pk.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T12:40:29Z","receivedAt":"2012-12-17T12:40:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2012 at 12:40:55AM -0800, Junio C Hamano wrote:\n\n> +# %C(auto,...) should trump --color=always\n> +#\n> +# NEEDSWORK: --color=never should also be tested but we need to run a\n> +# similar test under pseudo-terminal with test_terminal which is too\n> +# much hassle for its worth.\n> +\n> +test_format advanced-colors-forced \\\n> +\t'%C(auto,red yellow bold)foo%C(auto,reset)' --color=always <<'EOF'\n> +commit 131a310eb913d107dd3c09a65d1651175898735d\n> +foo\n> +commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873\n> +foo\n> +EOF\n\nHmm. I would have expected this to output colors. In other words, for\n\"auto\" to work just like the config-respecting colorization that is\nbuilt into git.\n\nYes, in this toy example, one could always just drop the \"auto\" when\nusing --color=always. But part of the point of this is that I could do:\n\n git config pretty.fake-oneline \"%C(auto,yellow)%h%C(auto,reset) %s\"\n\nand have it behave like \"--oneline\", no matter what the user has in\ntheir color.ui config or --color option on the command line.\n\nI think the patch below (on top of yours) does the right thing by\ncopying the color option from the rev_info diff options into the\npretty-print context, which is the same place that the graph and\nlog-tree code look. That means you'll get consistent colorization\n(either all or nothing) with:\n\n git log --graph -p --format='%C(auto,blue)%s%C(auto,reset)'\n\nno matter what your setting of color.diff, color.ui, or --color on the\ncommand line.\n\nIt also means that pretty-print defaults to \"no colors, do not even\ncheck stdout\" when people initialize it to all-zeroes. That's probably a\ngood thing, as it means any callers of format_commit_message have to\nconsciously opt into allowing colorization in their format messages.\n\ndiff --git a/commit.h b/commit.h\nindex b6ad8f3..0f469e5 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -89,6 +89,7 @@ struct pretty_print_context {\n \tchar *notes_message;\n \tstruct reflog_walk_info *reflog_info;\n \tconst char *output_encoding;\n+\tint color;\n };\n \n struct userformat_want {\ndiff --git a/log-tree.c b/log-tree.c\nindex 4f86def..8876c73 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)\n \tctx.preserve_subject = opt->preserve_subject;\n \tctx.reflog_info = opt->reflog_info;\n \tctx.fmt = opt->commit_format;\n+\tctx.color = opt->diffopt.use_color;\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n \n \tif (opt->add_signoff)\ndiff --git a/pretty.c b/pretty.c\nindex 9e51fec..e6a2886 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -967,7 +967,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tif (!end)\n \t\t\t\treturn 0;\n \t\t\tif (!memcmp(begin, \"auto,\", 5)) {\n-\t\t\t\tif (!want_color(-1))\n+\t\t\t\tif (!want_color(c->pretty_ctx->color))\n \t\t\t\t\treturn end - placeholder + 1;\n \t\t\t\tbegin += 5;\n \t\t\t}\n"},{"id":"205064","messageId":"7v4njkmq07.fsf@alter.siamese.dyndns.org","threadId":"32324","inReplyTo":"20121217121354.GB21858@sigill.intra.peff.net","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T19:34:48Z","receivedAt":"2012-12-17T19:34:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Dec 17, 2012 at 06:44:10PM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>> > if (!end)\n>> > return 0;\n>> > - color_parse_mem(placeholder + 2,\n>> > - end - (placeholder + 2),\n>> > + if (!memcmp(begin, \"auto,\", 5)) {\n>> > + if (!want_color(-1))\n>> > + return end - placeholder + 1;\n>> \n>> This want_color() checks color.ui and only when color.ui = auto, it\n>> bothers to check if the output is tty. I think the document should say\n>> that \"auto,\" (or maybe another name because it's not really auto)\n>> respects color.ui.\n>\n> Yeah, that should definitely be documented. I wonder if it should\n> actually respect color.diff, which is what \"log\" usually uses (albeit\n> mostly for the diff itself, we have always used it for the graph and for\n> the \"commit\" header of each entry).\n\nI actually do not like this patch very much. The original motive\nbehind this \"auto\" thing was to relieve the script writers from\nthe burden of having to write:\n\n\tif tty -s\n then\n \twarn='%C(red)'\n reset='%C(reset)'\n\telse\n \twarn= reset=\n\tfi\n fmt=\"${warn}WARNING: boo${reset} %s\"\n\nand instead let them write:\n\n\tfmt=\"%C(auto,red)WARNING: boo%C(auto,reset) %s\"\n\nbut between the two, there is no $cmd.color configuration involved\nin the first place. I am not sure what $cmd.color configuration\nshould take effect---perhaps for a \"git frotz\" script, we should\nallow the script writer to honor frotz.color=(auto,never,always)\nconfiguration, not just ui.color variable.\n\nAlso the patch as posted deliberately omits support to honor command\nline override --color=(auto,never,always), but it may be more\nnatural to expect\n\n git show --format='%C(auto,red)%s%C(auto,reset)' --color=never\n\nto defeat the \"auto,\" part the script writer wrote.\n\nNow, such a script would be run by its end users as\n\n $ git frotz --color=never\n\nIt has to do its own option parsing before running the underlying\n\"git show\" to decide if it passes \"--color=never\" from the command\nline for that to happen.\n\nBut at that point, we are back to the square one. Such a script\nwould be doing something like:\n\n\tif cmdline_has_color_flag\n then\n\t\tuse_color=... that flag ...\n\telif git config --get-colorbool frotz.color\n\tthen\n\t\tuse_color=--color=always\n\telse\n\t\tuse_color=--color=never\n\tfi\n\nin its early part to decide $use_color, to be used in the call it\nmakes to \"git show\" later on:\n\n\tgit show --format=\"$fmt\" $use_color\n\nAdding the logic to decide if %C(...) should be added to $fmt no\nlonger is an additional burden to the script writer, making the\nwhole %C(auto,red) machinery of little use.\n\nSo...\n"},{"id":"205066","messageId":"20121217194926.GA5209@sigill.intra.peff.net","threadId":"32324","inReplyTo":"7v4njkmq07.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T19:49:26Z","receivedAt":"2012-12-17T19:49:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2012 at 11:34:48AM -0800, Junio C Hamano wrote:\n\n> > Yeah, that should definitely be documented. I wonder if it should\n> > actually respect color.diff, which is what \"log\" usually uses (albeit\n> > mostly for the diff itself, we have always used it for the graph and for\n> > the \"commit\" header of each entry).\n> \n> I actually do not like this patch very much. The original motive\n> behind this \"auto\" thing was to relieve the script writers from\n> the burden of having to write:\n> \n> \tif tty -s\n> then\n> \twarn='%C(red)'\n> reset='%C(reset)'\n> \telse\n> \twarn= reset=\n> \tfi\n> fmt=\"${warn}WARNING: boo${reset} %s\"\n> \n> and instead let them write:\n> \n> \tfmt=\"%C(auto,red)WARNING: boo%C(auto,reset) %s\"\n> \n> but between the two, there is no $cmd.color configuration involved\n> in the first place. I am not sure what $cmd.color configuration\n> should take effect---perhaps for a \"git frotz\" script, we should\n> allow the script writer to honor frotz.color=(auto,never,always)\n> configuration, not just ui.color variable.\n\nSince this is by definition just talking about the log traversal, I\nthink it makes sense to respect the log traversal option by default\n(which is confusingly color.diff, of course, but that is a separate\nissue). That means that scripts which just wrap a regular traversal will\ndo what the user is accustomed to.\n\nIf \"git frotz\" wants to have a separate \"color.frotz\" option to override\nthat, then they would need to implement that themselves either with or\nwithout your patch. I do not think its presence makes things any harder.\n\n> Also the patch as posted deliberately omits support to honor command\n> line override --color=(auto,never,always), but it may be more\n> natural to expect\n> \n> git show --format='%C(auto,red)%s%C(auto,reset)' --color=never\n> \n> to defeat the \"auto,\" part the script writer wrote.\n> \n> Now, such a script would be run by its end users as\n> \n> $ git frotz --color=never\n> \n> It has to do its own option parsing before running the underlying\n> \"git show\" to decide if it passes \"--color=never\" from the command\n> line for that to happen.\n\nYeah, _if_ it does not just pass its options directly to git-log. Which\nI think a reasonable number of scripts do, as well as pretty.* aliases\n(which are not really scripts, but have the same relationship with\nrespect to this feature). For example, \"git stash list\" does not use\ncolor in its output, but could be improved to do so after your patch.\n\nSo no, I do not think you can cover every conceivable case. But having\ngit-log respect --color and the usual color.* variables for this feature\nseems like the only sane default. It makes the easy cases just work, and\nthe hard cases are not any worse off (and they may even be better off,\nsince the script can manipulate --color instead of rewriting their\nformat strings, but that is a minor difference).\n\n-Peff\n"},{"id":"205069","messageId":"7vmwxcla3n.fsf@alter.siamese.dyndns.org","threadId":"32324","inReplyTo":"20121217194926.GA5209@sigill.intra.peff.net","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T20:03:40Z","receivedAt":"2012-12-17T20:03:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If \"git frotz\" wants to have a separate \"color.frotz\" option to override\n> that, then they would need to implement that themselves either with or\n> without your patch. I do not think its presence makes things any harder.\n\nThat _was_ (but no longer is) exactly my point. Eh, rather, its\nabsense does not make things any harder.\n\n> So no, I do not think you can cover every conceivable case. But having\n> git-log respect --color and the usual color.* variables for this feature\n> seems like the only sane default. It makes the easy cases just work, and\n> the hard cases are not any worse off (and they may even be better off,\n> since the script can manipulate --color instead of rewriting their\n> format strings, but that is a minor difference).\n\nOK, care to reroll the one with your patch in the other message\nsquashed in, possibly with fixes to the test (the result should now\nhonor --color={always,never}, I think)?\n\nThanks.\n"},{"id":"205085","messageId":"20121217225516.GA1408@sigill.intra.peff.net","threadId":"32324","inReplyTo":"7vmwxcla3n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T22:55:16Z","receivedAt":"2012-12-17T22:55:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2012 at 12:03:40PM -0800, Junio C Hamano wrote:\n\n> > So no, I do not think you can cover every conceivable case. But having\n> > git-log respect --color and the usual color.* variables for this feature\n> > seems like the only sane default. It makes the easy cases just work, and\n> > the hard cases are not any worse off (and they may even be better off,\n> > since the script can manipulate --color instead of rewriting their\n> > format strings, but that is a minor difference).\n> \n> OK, care to reroll the one with your patch in the other message\n> squashed in, possibly with fixes to the test (the result should now\n> honor --color={always,never}, I think)?\n\nHere it is. It was easier to just skip using test_format, so it did not\nneed to be touched. I broke its cleanups out into a separate patch.\n\n [1/2]: t6006: clean up whitespace\n [2/2]: log --format: teach %C(auto,black) to respect color config\n\n-Peff\n"},{"id":"205090","messageId":"20121217225552.GA1653@sigill.intra.peff.net","threadId":"32324","inReplyTo":"20121217225516.GA1408@sigill.intra.peff.net","subject":"[PATCH 1/2] t6006: clean up whitespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-17T22:55:52Z","receivedAt":"2012-12-17T22:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The test_format function did not indent its in-line test\nscript in an attempt to make the output of the test look\nbetter. But it does not make a big difference to the output,\nand the source looks quite ugly. Let's use our normal\nindenting instead.\n\nSigned-off-by: Jeff King \n---\n t/t6006-rev-list-format.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex f94f0c4..c0c62c9 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -11,12 +11,12 @@ test_format() {\n '\n \n # usage: test_format name format_string expect.$1\n \ttest_expect_success \"format $1\" \"\n-git rev-list --pretty=format:'$2' master >output.$1 &&\n-test_cmp expect.$1 output.$1\n-\"\n+\t\tgit rev-list --pretty=format:'$2' master >output.$1 &&\n+\t\ttest_cmp expect.$1 output.$1\n+\t\"\n }\n \n test_format percent %%h <<'EOF'\n-- \n1.8.1.rc2.28.gce0611a\n"},{"id":"205086","messageId":"20121217225623.GB1653@sigill.intra.peff.net","threadId":"32324","inReplyTo":"20121217225516.GA1408@sigill.intra.peff.net","subject":"[PATCH 1/2] t6006: clean up whitespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T22:56:23Z","receivedAt":"2012-12-17T22:56:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Junio C Hamano \n\nThe test_format function did not indent its in-line test\nscript in an attempt to make the output of the test look\nbetter. But it does not make a big difference to the output,\nand the source looks quite ugly. Let's use our normal\nindenting instead.\n\nSigned-off-by: Jeff King \n---\n t/t6006-rev-list-format.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex f94f0c4..c0c62c9 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -11,12 +11,12 @@ test_format() {\n '\n \n # usage: test_format name format_string expect.$1\n \ttest_expect_success \"format $1\" \"\n-git rev-list --pretty=format:'$2' master >output.$1 &&\n-test_cmp expect.$1 output.$1\n-\"\n+\t\tgit rev-list --pretty=format:'$2' master >output.$1 &&\n+\t\ttest_cmp expect.$1 output.$1\n+\t\"\n }\n \n test_format percent %%h <<'EOF'\n-- \n1.8.1.rc2.28.gce0611a\n"},{"id":"205087","messageId":"20121217225649.GC1653@sigill.intra.peff.net","threadId":"32324","inReplyTo":"20121217225516.GA1408@sigill.intra.peff.net","subject":"[PATCH 2/2] log --format: teach %C(auto,black) to respect color config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T22:56:49Z","receivedAt":"2012-12-17T22:56:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Junio C Hamano \n\nTraditionally, %C(color attr) always emitted the ANSI color\nsequence; it was up to the scripts that wanted to conditionally\ncolor their output to omit %C(...) specifier when they do not want\ncolors.\n\nOptionally allow \"auto,\" to be prefixed to the color, so that the\noutput is colored iff we would color regular \"log\" output\n(e.g., taking into account color.* and --color command line\noptions).\n\nTests and pretty_context bits by Jeff King .\n\nSigned-off-by: Jeff King \n---\n Documentation/pretty-formats.txt | 6 ++++-\n commit.h | 1 +\n log-tree.c | 1 +\n pretty.c | 13 +++++++---\n t/t6006-rev-list-format.sh | 55 ++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 72 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex d9edded..105f18a 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -144,7 +144,11 @@ The placeholders are:\n - '%Cgreen': switch color to green\n - '%Cblue': switch color to blue\n - '%Creset': reset color\n-- '%C(...)': color specification, as described in color.branch.* config option\n+- '%C(...)': color specification, as described in color.branch.* config option;\n+ adding `auto,` at the beginning will emit color only when colors are\n+ enabled for log output (by `color.diff`, `color.ui`, or `--color`, and\n+ respecting the `auto` settings of the former if we are going to a\n+ terminal)\n - '%m': left, right or boundary mark\n - '%n': newline\n - '%%': a raw '%'\ndiff --git a/commit.h b/commit.h\nindex b6ad8f3..0f469e5 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -89,6 +89,7 @@ struct pretty_print_context {\n \tchar *notes_message;\n \tstruct reflog_walk_info *reflog_info;\n \tconst char *output_encoding;\n+\tint color;\n };\n \n struct userformat_want {\ndiff --git a/log-tree.c b/log-tree.c\nindex 4f86def..8876c73 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)\n \tctx.preserve_subject = opt->preserve_subject;\n \tctx.reflog_info = opt->reflog_info;\n \tctx.fmt = opt->commit_format;\n+\tctx.color = opt->diffopt.use_color;\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n \n \tif (opt->add_signoff)\ndiff --git a/pretty.c b/pretty.c\nindex 5bdc2e7..e6a2886 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tswitch (placeholder[0]) {\n \tcase 'C':\n \t\tif (placeholder[1] == '(') {\n-\t\t\tconst char *end = strchr(placeholder + 2, ')');\n+\t\t\tconst char *begin = placeholder + 2;\n+\t\t\tconst char *end = strchr(begin, ')');\n \t\t\tchar color[COLOR_MAXLEN];\n+\n \t\t\tif (!end)\n \t\t\t\treturn 0;\n-\t\t\tcolor_parse_mem(placeholder + 2,\n-\t\t\t\t\tend - (placeholder + 2),\n+\t\t\tif (!memcmp(begin, \"auto,\", 5)) {\n+\t\t\t\tif (!want_color(c->pretty_ctx->color))\n+\t\t\t\t\treturn end - placeholder + 1;\n+\t\t\t\tbegin += 5;\n+\t\t\t}\n+\t\t\tcolor_parse_mem(begin,\n+\t\t\t\t\tend - begin,\n \t\t\t\t\t\"--pretty format\", color);\n \t\t\tstrbuf_addstr(sb, color);\n \t\t\treturn end - placeholder + 1;\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex c0c62c9..3fc3b74 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -3,6 +3,7 @@ test_description='git rev-list --pretty=format test'\n test_description='git rev-list --pretty=format test'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n test_tick\n test_expect_success 'setup' '\n@@ -19,6 +20,18 @@ test_format () {\n \t\"\n }\n \n+# Feed to --format to provide predictable colored sequences.\n+AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'\n+has_color () {\n+\tprintf '\\033[31mfoo\\033[m\\n' >expect &&\n+\ttest_cmp expect \"$1\"\n+}\n+\n+has_no_color () {\n+\techo foo >expect &&\n+\ttest_cmp expect \"$1\"\n+}\n+\n test_format percent %%h <<'EOF'\n commit 131a310eb913d107dd3c09a65d1651175898735d\n %h\n@@ -124,6 +137,48 @@ EOF\n \u001b[1;31;43mfoo\u001b[m\n EOF\n \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+\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+\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+\tgit log --format=$AUTO_COLOR -1 --color >actual &&\n+\thas_color actual\n+'\n+\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+\t(\n+\t\tTERM=vt100 && export TERM &&\n+\t\ttest_terminal \\\n+\t\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n+\t\thas_color actual\n+\t)\n+'\n+\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+\t\thas_no_color actual\n+\t)\n+'\n+\n cat >commit-msg <<'EOF'\n Test printing of complex bodies\n \n-- \n1.8.1.rc2.28.gce0611a\n"},{"id":"205089","messageId":"20121217225958.GA1809@sigill.intra.peff.net","threadId":"32324","inReplyTo":"20121217225552.GA1653@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t6006: clean up whitespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T22:59:58Z","receivedAt":"2012-12-17T22:59:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 17, 2012 at 05:55:52PM -0500, Junio C Hamano wrote:\n\n> From: Junio C Hamano \n> To: Junio C Hamano \n>\n> The test_format function did not indent its in-line test\n> script in an attempt to make the output of the test look\n> better. But it does not make a big difference to the output,\n> and the source looks quite ugly. Let's use our normal\n> indenting instead.\n> \n> Signed-off-by: Jeff King \n\nArgh. This is me accidentally impersonating Junio because my home-grown\nsend-email replacement does not grok patches by other authors properly.\nSorry for the noise; I've just resent with a proper header.\n\nI should probably fix my script before I embarrass myself again with\nit...\n\n-Peff\n"}]}