Re: [PATCH] Pretty formats: skip color codes if !want_color()

From: Jeff King <peff@peff.net>
Date: Dec 17, 2012, 23:10 UTC
Message-ID: <20121217231042.GB1809@sigill.intra.peff.net>
In-Reply-To: <20121217225703.GC25103@orenhe-laptop>

On Tue, Dec 18, 2012 at 12:57:03AM +0200, Oren Held wrote:

> Avoid color escape codes if colors are disabled, just like the
> behavior of other git commands.  This solves the case of color escape
> codes in stdout when piping or redirecting, e.g.: $ git log
> --format=%Cred%h > out

You may be interested in this thread from today, which is attacking the same problem:

  http://thread.gmane.org/gmane.comp.version-control.git/211370/focus=211714

Two issues with your patch:

Show 6 quoted lines

> @@ -956,6 +962,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
>  	struct commit_list *p;
>  	int h1, h2;
>  
> +	int colors_enabled = want_color(GIT_COLOR_UNKNOWN);
> +

I think this would want to use the regular diff color variable to be consistent with other coloring of "git log".

Show 8 quoted lines

> @@ -967,20 +975,20 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
>  			color_parse_mem(placeholder + 2,
>  					end - (placeholder + 2),
>  					"--pretty format", color);
> -			strbuf_addstr(sb, color);
> +			conditional_strbuf_addstr(colors_enabled, sb, color);
>  			return end - placeholder + 1;
>  		}

This breaks backwards compatibility for callers who expect the coloring to be unconditional; adding new syntax would solve that.

The patch I linked to above solves both.

-Peff