Re: [PATCH 2/2] expand --pretty=format color options

[Jeff King <peff@peff.net>]

On Sun, Jan 18, 2009 at 12:37:53PM -0500, Jeff King wrote:

> On Sun, Jan 18, 2009 at 06:13:21PM +0100, René Scharfe wrote:
> > Another step would be for --pretty=format to respect the color config,
> > i.e. it shouldn't print any colour codes if colouring is turned off or
> > if set to auto while writing to file or pipe.
> 
> That makes sense to me. In theory, we could offer an "always use this
> color" and a "conditionally use this color" substitution. But I don't
> really see why anyone would want the "always use this color" one (they
> could just say --color, then).

Here is a patch that seems to work. It predicates pretty format colors
on diff colors, which is the same way the yellow commit header works in
log-tree. I don't know if it makes more sense to introduce yet another
color config option.

And I say "seems to work" because I remember there being some trickery
with color flags sometimes not getting set properly. However, since this
is the same flag as the yellow commit header, and called around the same
time, I think it should be fine.

One final note: if you are writing "generic" format strings, you
probably don't want to say "yellow". You want to say "whatever the user
has configured for color.diff.commit". I don't know if %C should be
overloaded for that or not (i.e., %C(commit) would be a valid color).

-- >8 --
respect color settings for --pretty=format colors

Previously, we would unconditionally print any colors the
user put into the --pretty=format specifier, regardless of
the setting of color configuration. This is not a problem if
the user writes a custom string each time git-log is
invoked. However, it makes use in scripts unnecessarily
complex, since they would have to change the format string
based on user preferences (and whether output is going to a
tty).

This patch will ignore any colors in --pretty=format if diff
colors are not turned on (they will still be parsed and
treated as placeholders, but no color will be output). This
matches the color.diff.commit colorization, which depends on
diff coloring even though it is not technically part of a
diff.
---
 pretty.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/pretty.c b/pretty.c
index b1b8620..fe606c5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -563,20 +563,25 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 			color_parse_mem(placeholder + 2,
 					end - (placeholder + 2),
 					"--pretty format", color);
-			strbuf_addstr(sb, color);
+			if (diff_use_color_default)
+				strbuf_addstr(sb, color);
 			return end - placeholder + 1;
 		}
 		if (!prefixcmp(placeholder + 1, "red")) {
-			strbuf_addstr(sb, "\033[31m");
+			if (diff_use_color_default)
+				strbuf_addstr(sb, "\033[31m");
 			return 4;
 		} else if (!prefixcmp(placeholder + 1, "green")) {
-			strbuf_addstr(sb, "\033[32m");
+			if (diff_use_color_default)
+				strbuf_addstr(sb, "\033[32m");
 			return 6;
 		} else if (!prefixcmp(placeholder + 1, "blue")) {
-			strbuf_addstr(sb, "\033[34m");
+			if (diff_use_color_default)
+				strbuf_addstr(sb, "\033[34m");
 			return 5;
 		} else if (!prefixcmp(placeholder + 1, "reset")) {
-			strbuf_addstr(sb, "\033[m");
+			if (diff_use_color_default)
+				strbuf_addstr(sb, "\033[m");
 			return 6;
 		} else
 			return 0;
-- 
1.6.1.151.g5a7da.dirty