{"thread":{"id":"32380","subject":"[PATCH] Pretty formats: skip color codes if !want_color()","startedAt":"2012-12-17T22:57:03Z","lastAt":"2012-12-17T23:10:42Z","messageCount":2,"participants":["Oren Held","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"205088","messageId":"20121217225703.GC25103@orenhe-laptop","threadId":"32380","inReplyTo":null,"subject":"[PATCH] Pretty formats: skip color codes if !want_color()","fromName":"Oren Held","fromEmail":"oren@held.org.il","sentAt":"2012-12-17T22:57:03Z","receivedAt":"2012-12-17T22:57:03Z","isPatch":true,"sender":{"key":"oren@held.org.il","avatar":null},"body":"Avoid color escape codes if colors are disabled, just like the behavior of other git commands.\nThis solves the case of color escape codes in stdout when piping or redirecting, e.g.:\n$ git log --format=%Cred%h > out\n\nSigned-off-by: Oren Held <oren@held.org.il>\n---\nWould appreciate your help or comments :)\n\n pretty.c |   18 +++++++++++++-----\n 1 file changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 5bdc2e7..9637dfd 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -947,6 +947,12 @@ static int format_reflog_person(struct strbuf *sb,\n \treturn format_person_part(sb, part, ident, strlen(ident), dmode);\n }\n \n+static void conditional_strbuf_addstr(int conditional, struct strbuf *sb,\n+\t\t\t\t      const char *s) {\n+\tif (conditional)\n+\t\tstrbuf_addstr(sb, s);\n+}\n+\n static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\t\tvoid *context)\n {\n@@ -956,6 +962,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \tstruct commit_list *p;\n \tint h1, h2;\n \n+\tint colors_enabled = want_color(GIT_COLOR_UNKNOWN);\n+\n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n \tcase 'C':\n@@ -967,20 +975,20 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tcolor_parse_mem(placeholder + 2,\n \t\t\t\t\tend - (placeholder + 2),\n \t\t\t\t\t\"--pretty format\", color);\n-\t\t\tstrbuf_addstr(sb, color);\n+\t\t\tconditional_strbuf_addstr(colors_enabled, sb, color);\n \t\t\treturn end - placeholder + 1;\n \t\t}\n \t\tif (!prefixcmp(placeholder + 1, \"red\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_RED);\n+\t\t\tconditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_RED);\n \t\t\treturn 4;\n \t\t} else if (!prefixcmp(placeholder + 1, \"green\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_GREEN);\n+\t\t\tconditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_GREEN);\n \t\t\treturn 6;\n \t\t} else if (!prefixcmp(placeholder + 1, \"blue\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_BLUE);\n+\t\t\tconditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_BLUE);\n \t\t\treturn 5;\n \t\t} else if (!prefixcmp(placeholder + 1, \"reset\")) {\n-\t\t\tstrbuf_addstr(sb, GIT_COLOR_RESET);\n+\t\t\tconditional_strbuf_addstr(colors_enabled, sb, GIT_COLOR_RESET);\n \t\t\treturn 6;\n \t\t} else\n \t\t\treturn 0;\n-- \n1.7.10.4\n"},{"id":"205091","messageId":"20121217231042.GB1809@sigill.intra.peff.net","threadId":"32380","inReplyTo":"20121217225703.GC25103@orenhe-laptop","subject":"Re: [PATCH] Pretty formats: skip color codes if !want_color()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-17T23:10:42Z","receivedAt":"2012-12-17T23:10:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 18, 2012 at 12:57:03AM +0200, Oren Held wrote:\n\n> Avoid color escape codes if colors are disabled, just like the\n> behavior of other git commands.  This solves the case of color escape\n> codes in stdout when piping or redirecting, e.g.: $ git log\n> --format=%Cred%h > out\n\nYou may be interested in this thread from today, which is attacking the\nsame problem:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/211370/focus=211714\n\nTwo issues with your patch:\n\n> @@ -956,6 +962,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n>  \tstruct commit_list *p;\n>  \tint h1, h2;\n>  \n> +\tint colors_enabled = want_color(GIT_COLOR_UNKNOWN);\n> +\n\nI think this would want to use the regular diff color variable to be\nconsistent with other coloring of \"git log\".\n\n> @@ -967,20 +975,20 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n>  \t\t\tcolor_parse_mem(placeholder + 2,\n>  \t\t\t\t\tend - (placeholder + 2),\n>  \t\t\t\t\t\"--pretty format\", color);\n> -\t\t\tstrbuf_addstr(sb, color);\n> +\t\t\tconditional_strbuf_addstr(colors_enabled, sb, color);\n>  \t\t\treturn end - placeholder + 1;\n>  \t\t}\n\nThis breaks backwards compatibility for callers who expect the coloring\nto be unconditional; adding new syntax would solve that.\n\nThe patch I linked to above solves both.\n\n-Peff\n"}]}