FW: Git log --graph doesn't output color when redirected

FW: Git log --graph doesn't output color when redirected

[Srb, Michal]

Unlike --pretty-format, --graph doesn’t output colors when the git log output
is redirected.
 
Tested on Ubuntu 12.04 and msys on Windows 8.
 
Is there a setting somewhere in config to change this?
 
Thanks,
 
Michal

Re: FW: Git log --graph doesn't output color when redirected

[Jeff King]

On Wed, Dec 12, 2012 at 05:35:17PM +0000, Srb, Michal wrote:

> Unlike --pretty-format, --graph doesn’t output colors when the git log output
> is redirected.

I do not think it has anything to do with --graph in particular, but
rather that when colorization is set to the "auto" mode, it is enabled
only when stdout is a tty. Diff coloring, for example, follows the same
rules.  If you are using --format="%C(red)" or similar placeholders,
they are the odd duck by not respecting the auto-color mode.

> Is there a setting somewhere in config to change this?

Yes. If you use "--color" on the command line, that means
"unconditionally use color". If you set color.ui (or any other color
config option) to "always", then you will always get color (and you can
disable it for a particular run with "--no-color"). Setting a color
config option to "true" is the same as "auto", which gets you the auto
mode.

-Peff

RE: FW: Git log --graph doesn't output color when redirected

[Srb, Michal]

From: Jeff King [peff@peff.net]
Sent: Thursday, December 13, 2012 1:13 PM

>> Is there a setting somewhere in config to change this?

> Yes. If you use "--color" on the command line, that means
> "unconditionally use color". If you set color.ui (or any other
> color config option) to "always", then you will always get color (and
> you can disable it for a particular run with "--no-color"). Setting a color
> config option to "true" is the same as "auto", which gets you 
> the auto mode.
 
Setting color in gitconfig didn't work for me on msys, but --color 
works like magic, thanks!

Re: FW: Git log --graph doesn't output color when redirected

[Nguyen Thai Ngoc Duy]

On Thu, Dec 13, 2012 at 8:13 PM, Jeff King <peff@peff.net> wrote:
> If you are using --format="%C(red)" or similar placeholders,
> they are the odd duck by not respecting the auto-color mode.

But they should, shouldn't they? Just asking. I may do it to when I
revive nd/pretty-placeholder-with-color-option.
-- 
Duy

Re: FW: Git log --graph doesn't output color when redirected

[Jeff King]

On Sat, Dec 15, 2012 at 10:23:10AM +0700, Nguyen Thai Ngoc Duy wrote:

> On Thu, Dec 13, 2012 at 8:13 PM, Jeff King <peff@peff.net> wrote:
> > If you are using --format="%C(red)" or similar placeholders,
> > they are the odd duck by not respecting the auto-color mode.
> 
> But they should, shouldn't they? Just asking. I may do it to when I
> revive nd/pretty-placeholder-with-color-option.

If I were designing --format today, I would certainly say so. The only
thing holding me back would be backwards compatibility. We could get
around that by introducing a new placeholder like %c(color) that behaves
like %C(color), except respects the --color flag.

It looks like this came up before:

  http://article.gmane.org/gmane.comp.version-control.git/169225

but never got pushed to inclusion.

-Peff

Re: FW: Git log --graph doesn't output color when redirected

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Dec 15, 2012 at 10:23:10AM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> On Thu, Dec 13, 2012 at 8:13 PM, Jeff King <peff@peff.net> wrote:
>> > If you are using --format="%C(red)" or similar placeholders,
>> > they are the odd duck by not respecting the auto-color mode.
>> 
>> But they should, shouldn't they? Just asking. I may do it to when I
>> revive nd/pretty-placeholder-with-color-option.
>
> If I were designing --format today, I would certainly say so. The only
> thing holding me back would be backwards compatibility. We could get
> around that by introducing a new placeholder like %c(color) that behaves
> like %C(color), except respects the --color flag.

I think the %c(color) thing is a good way to go if we want to pursue
this.

Another possibility without wasting one more special letter would be
to allow %C(auto,red), perhaps like this (untested):

 pretty.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git i/pretty.c w/pretty.c
index dba6828..77cf826 100644
--- i/pretty.c
+++ w/pretty.c
@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 	switch (placeholder[0]) {
 	case 'C':
 		if (placeholder[1] == '(') {
-			const char *end = strchr(placeholder + 2, ')');
+			const char *begin = placeholder + 2;
+			const char *end = strchr(begin, ')');
 			char color[COLOR_MAXLEN];
+
 			if (!end)
 				return 0;
-			color_parse_mem(placeholder + 2,
-					end - (placeholder + 2),
+			if (!memcmp(begin, "auto,", 5)) {
+				if (!want_color(GIT_COLOR_AUTO))
+					return 0;
+				begin += 5;
+			}
+			color_parse_mem(begin,
+					end - begin,
 					"--pretty format", color);
 			strbuf_addstr(sb, color);
 			return end - placeholder + 1;

[PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Junio C Hamano]

Traditionally, %C(color attr) always emitted the ANSI color
sequence; it was up to the scripts that wanted to conditionally
color their output to omit %C(...) specifier when they do not want
colors.

Optionally allow "auto," to be prefixed to the color, so that the
output is colored iff it goes to the terminal.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This time with minimum test and documentation.

 Documentation/pretty-formats.txt |  4 +++-
 pretty.c                         | 13 ++++++++++---
 t/t6006-rev-list-format.sh       | 30 ++++++++++++++++++++++++++----
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d9edded..2af017c 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -144,7 +144,9 @@ The placeholders are:
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
 - '%Creset': reset color
-- '%C(...)': color specification, as described in color.branch.* config option
+- '%C(...)': color specification, as described in color.branch.* config option;
+  adding `auto,` at the beginning will emit color only when the
+  output is going to a terminal
 - '%m': left, right or boundary mark
 - '%n': newline
 - '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index dba6828..b9fd972 100644
--- a/pretty.c
+++ b/pretty.c
@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 	switch (placeholder[0]) {
 	case 'C':
 		if (placeholder[1] == '(') {
-			const char *end = strchr(placeholder + 2, ')');
+			const char *begin = placeholder + 2;
+			const char *end = strchr(begin, ')');
 			char color[COLOR_MAXLEN];
+
 			if (!end)
 				return 0;
-			color_parse_mem(placeholder + 2,
-					end - (placeholder + 2),
+			if (!memcmp(begin, "auto,", 5)) {
+				if (!want_color(-1))
+					return end - placeholder + 1;
+				begin += 5;
+			}
+			color_parse_mem(begin,
+					end - begin,
 					"--pretty format", color);
 			strbuf_addstr(sb, color);
 			return end - placeholder + 1;
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f94f0c4..bfcc1c6 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -11,12 +11,12 @@ touch foo && git add foo && git commit -m "added foo" &&
 '
 
 # usage: test_format name format_string <expected_output
-test_format() {
+test_format () {
 	cat >expect.$1
 	test_expect_success "format $1" "
-git rev-list --pretty=format:'$2' master >output.$1 &&
-test_cmp expect.$1 output.$1
-"
+		git rev-list --pretty=format:'$2'${3:+ $3} master >output.$1 &&
+		test_cmp expect.$1 output.$1
+	"
 }
 
 test_format percent %%h <<'EOF'
@@ -124,6 +124,28 @@ commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
 ^[[1;31;43mfoo^[[m
 EOF
 
+test_format advanced-colors-auto \
+	'%C(auto,red yellow bold)foo%C(auto,reset)' --color <<'EOF'
+commit 131a310eb913d107dd3c09a65d1651175898735d
+foo
+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+foo
+EOF
+
+# %C(auto,...) should trump --color=always
+#
+# NEEDSWORK: --color=never should also be tested but we need to run a
+# similar test under pseudo-terminal with test_terminal which is too
+# much hassle for its worth.
+
+test_format advanced-colors-forced \
+	'%C(auto,red yellow bold)foo%C(auto,reset)' --color=always <<'EOF'
+commit 131a310eb913d107dd3c09a65d1651175898735d
+foo
+commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+foo
+EOF
+
 cat >commit-msg <<'EOF'
 Test printing of complex bodies
 
-- 
1.8.1.rc2.126.ge9b7782

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Nguyen Thai Ngoc Duy]

On Mon, Dec 17, 2012 at 3:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Traditionally, %C(color attr) always emitted the ANSI color
> sequence; it was up to the scripts that wanted to conditionally
> color their output to omit %C(...) specifier when they do not want
> colors.
>
> Optionally allow "auto," to be prefixed to the color, so that the
> output is colored iff it goes to the terminal.

I see "prefix" is clearly documented. Still it feels a bit unnatural
that %C(red,auto) won't work. But we can make that case work later if
somebody cares enough.

>                         if (!end)
>                                 return 0;
> -                       color_parse_mem(placeholder + 2,
> -                                       end - (placeholder + 2),
> +                       if (!memcmp(begin, "auto,", 5)) {
> +                               if (!want_color(-1))
> +                                       return end - placeholder + 1;

This want_color() checks color.ui and only when color.ui = auto, it
bothers to check if the output is tty. I think the document should say
that "auto," (or maybe another name because it's not really auto)
respects color.ui.
-- 
Duy

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Jeff King]

On Mon, Dec 17, 2012 at 06:44:10PM +0700, Nguyen Thai Ngoc Duy wrote:

> >                         if (!end)
> >                                 return 0;
> > -                       color_parse_mem(placeholder + 2,
> > -                                       end - (placeholder + 2),
> > +                       if (!memcmp(begin, "auto,", 5)) {
> > +                               if (!want_color(-1))
> > +                                       return end - placeholder + 1;
> 
> This want_color() checks color.ui and only when color.ui = auto, it
> bothers to check if the output is tty. I think the document should say
> that "auto," (or maybe another name because it's not really auto)
> respects color.ui.

Yeah, that should definitely be documented. I wonder if it should
actually respect color.diff, which is what "log" usually uses (albeit
mostly for the diff itself, we have always used it for the graph and for
the "commit" header of each entry).

-Peff

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Dec 17, 2012 at 06:44:10PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> >                         if (!end)
>> >                                 return 0;
>> > -                       color_parse_mem(placeholder + 2,
>> > -                                       end - (placeholder + 2),
>> > +                       if (!memcmp(begin, "auto,", 5)) {
>> > +                               if (!want_color(-1))
>> > +                                       return end - placeholder + 1;
>> 
>> This want_color() checks color.ui and only when color.ui = auto, it
>> bothers to check if the output is tty. I think the document should say
>> that "auto," (or maybe another name because it's not really auto)
>> respects color.ui.
>
> Yeah, that should definitely be documented. I wonder if it should
> actually respect color.diff, which is what "log" usually uses (albeit
> mostly for the diff itself, we have always used it for the graph and for
> the "commit" header of each entry).

I actually do not like this patch very much.  The original motive
behind this "auto" thing was to relieve the script writers from
the burden of having to write:

	if tty -s
        then
        	warn='%C(red)'
                reset='%C(reset)'
	else
        	warn= reset=
	fi
        fmt="${warn}WARNING: boo${reset} %s"

and instead let them write:

	fmt="%C(auto,red)WARNING: boo%C(auto,reset) %s"

but between the two, there is no $cmd.color configuration involved
in the first place.  I am not sure what $cmd.color configuration
should take effect---perhaps for a "git frotz" script, we should
allow the script writer to honor frotz.color=(auto,never,always)
configuration, not just ui.color variable.

Also the patch as posted deliberately omits support to honor command
line override --color=(auto,never,always), but it may be more
natural to expect

    git show --format='%C(auto,red)%s%C(auto,reset)' --color=never

to defeat the "auto," part the script writer wrote.

Now, such a script would be run by its end users as

    $ git frotz --color=never

It has to do its own option parsing before running the underlying
"git show" to decide if it passes "--color=never" from the command
line for that to happen.

But at that point, we are back to the square one.  Such a script
would be doing something like:

	if cmdline_has_color_flag
        then
		use_color=... that flag ...
	elif git config --get-colorbool frotz.color
	then
		use_color=--color=always
	else
		use_color=--color=never
	fi

in its early part to decide $use_color, to be used in the call it
makes to "git show" later on:

	git show --format="$fmt" $use_color

Adding the logic to decide if %C(...) should be added to $fmt no
longer is an additional burden to the script writer, making the
whole %C(auto,red) machinery of little use.

So...

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Jeff King]

On Mon, Dec 17, 2012 at 11:34:48AM -0800, Junio C Hamano wrote:

> > Yeah, that should definitely be documented. I wonder if it should
> > actually respect color.diff, which is what "log" usually uses (albeit
> > mostly for the diff itself, we have always used it for the graph and for
> > the "commit" header of each entry).
> 
> I actually do not like this patch very much.  The original motive
> behind this "auto" thing was to relieve the script writers from
> the burden of having to write:
> 
> 	if tty -s
>         then
>         	warn='%C(red)'
>                 reset='%C(reset)'
> 	else
>         	warn= reset=
> 	fi
>         fmt="${warn}WARNING: boo${reset} %s"
> 
> and instead let them write:
> 
> 	fmt="%C(auto,red)WARNING: boo%C(auto,reset) %s"
> 
> but between the two, there is no $cmd.color configuration involved
> in the first place.  I am not sure what $cmd.color configuration
> should take effect---perhaps for a "git frotz" script, we should
> allow the script writer to honor frotz.color=(auto,never,always)
> configuration, not just ui.color variable.

Since this is by definition just talking about the log traversal, I
think it makes sense to respect the log traversal option by default
(which is confusingly color.diff, of course, but that is a separate
issue). That means that scripts which just wrap a regular traversal will
do what the user is accustomed to.

If "git frotz" wants to have a separate "color.frotz" option to override
that, then they would need to implement that themselves either with or
without your patch. I do not think its presence makes things any harder.

> Also the patch as posted deliberately omits support to honor command
> line override --color=(auto,never,always), but it may be more
> natural to expect
> 
>     git show --format='%C(auto,red)%s%C(auto,reset)' --color=never
> 
> to defeat the "auto," part the script writer wrote.
> 
> Now, such a script would be run by its end users as
> 
>     $ git frotz --color=never
> 
> It has to do its own option parsing before running the underlying
> "git show" to decide if it passes "--color=never" from the command
> line for that to happen.

Yeah, _if_ it does not just pass its options directly to git-log. Which
I think a reasonable number of scripts do, as well as pretty.* aliases
(which are not really scripts, but have the same relationship with
respect to this feature). For example, "git stash list" does not use
color in its output, but could be improved to do so after your patch.

So no, I do not think you can cover every conceivable case. But having
git-log respect --color and the usual color.* variables for this feature
seems like the only sane default. It makes the easy cases just work, and
the hard cases are not any worse off (and they may even be better off,
since the script can manipulate --color instead of rewriting their
format strings, but that is a minor difference).

-Peff

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> If "git frotz" wants to have a separate "color.frotz" option to override
> that, then they would need to implement that themselves either with or
> without your patch. I do not think its presence makes things any harder.

That _was_ (but no longer is) exactly my point.  Eh, rather, its
absense does not make things any harder.

> So no, I do not think you can cover every conceivable case. But having
> git-log respect --color and the usual color.* variables for this feature
> seems like the only sane default. It makes the easy cases just work, and
> the hard cases are not any worse off (and they may even be better off,
> since the script can manipulate --color instead of rewriting their
> format strings, but that is a minor difference).

OK, care to reroll the one with your patch in the other message
squashed in, possibly with fixes to the test (the result should now
honor --color={always,never}, I think)?

Thanks.

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Jeff King]

On Mon, Dec 17, 2012 at 12:03:40PM -0800, Junio C Hamano wrote:

> > So no, I do not think you can cover every conceivable case. But having
> > git-log respect --color and the usual color.* variables for this feature
> > seems like the only sane default. It makes the easy cases just work, and
> > the hard cases are not any worse off (and they may even be better off,
> > since the script can manipulate --color instead of rewriting their
> > format strings, but that is a minor difference).
> 
> OK, care to reroll the one with your patch in the other message
> squashed in, possibly with fixes to the test (the result should now
> honor --color={always,never}, I think)?

Here it is. It was easier to just skip using test_format, so it did not
need to be touched. I broke its cleanups out into a separate patch.

  [1/2]: t6006: clean up whitespace
  [2/2]: log --format: teach %C(auto,black) to respect color config

-Peff

[PATCH 1/2] t6006: clean up whitespace

[Junio C Hamano]

The test_format function did not indent its in-line test
script in an attempt to make the output of the test look
better. But it does not make a big difference to the output,
and the source looks quite ugly. Let's use our normal
indenting instead.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t6006-rev-list-format.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f94f0c4..c0c62c9 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -11,12 +11,12 @@ test_format() {
 '
 
 # usage: test_format name format_string <expected_output
-test_format() {
+test_format () {
 	cat >expect.$1
 	test_expect_success "format $1" "
-git rev-list --pretty=format:'$2' master >output.$1 &&
-test_cmp expect.$1 output.$1
-"
+		git rev-list --pretty=format:'$2' master >output.$1 &&
+		test_cmp expect.$1 output.$1
+	"
 }
 
 test_format percent %%h <<'EOF'
-- 
1.8.1.rc2.28.gce0611a

Re: [PATCH 1/2] t6006: clean up whitespace

[Jeff King]

On Mon, Dec 17, 2012 at 05:55:52PM -0500, Junio C Hamano wrote:

> From: Junio C Hamano <gitster@pobox.com>
> To: Junio C Hamano <gitster@pobox.com>
>
> The test_format function did not indent its in-line test
> script in an attempt to make the output of the test look
> better. But it does not make a big difference to the output,
> and the source looks quite ugly. Let's use our normal
> indenting instead.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Argh. This is me accidentally impersonating Junio because my home-grown
send-email replacement does not grok patches by other authors properly.
Sorry for the noise; I've just resent with a proper header.

I should probably fix my script before I embarrass myself again with
it...

-Peff

[PATCH 1/2] t6006: clean up whitespace

[Jeff King]

From: Junio C Hamano <gitster@pobox.com>

The test_format function did not indent its in-line test
script in an attempt to make the output of the test look
better. But it does not make a big difference to the output,
and the source looks quite ugly. Let's use our normal
indenting instead.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t6006-rev-list-format.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index f94f0c4..c0c62c9 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -11,12 +11,12 @@ test_format() {
 '
 
 # usage: test_format name format_string <expected_output
-test_format() {
+test_format () {
 	cat >expect.$1
 	test_expect_success "format $1" "
-git rev-list --pretty=format:'$2' master >output.$1 &&
-test_cmp expect.$1 output.$1
-"
+		git rev-list --pretty=format:'$2' master >output.$1 &&
+		test_cmp expect.$1 output.$1
+	"
 }
 
 test_format percent %%h <<'EOF'
-- 
1.8.1.rc2.28.gce0611a

[PATCH 2/2] log --format: teach %C(auto,black) to respect color config

[Jeff King]

From: Junio C Hamano <gitster@pobox.com>

Traditionally, %C(color attr) always emitted the ANSI color
sequence; it was up to the scripts that wanted to conditionally
color their output to omit %C(...) specifier when they do not want
colors.

Optionally allow "auto," to be prefixed to the color, so that the
output is colored iff we would color regular "log" output
(e.g., taking into account color.* and --color command line
options).

Tests and pretty_context bits by Jeff King <peff@peff.net>.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/pretty-formats.txt |  6 ++++-
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 13 +++++++---
 t/t6006-rev-list-format.sh       | 55 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d9edded..105f18a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -144,7 +144,11 @@ The placeholders are:
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
 - '%Creset': reset color
-- '%C(...)': color specification, as described in color.branch.* config option
+- '%C(...)': color specification, as described in color.branch.* config option;
+  adding `auto,` at the beginning will emit color only when colors are
+  enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
+  respecting the `auto` settings of the former if we are going to a
+  terminal)
 - '%m': left, right or boundary mark
 - '%n': newline
 - '%%': a raw '%'
diff --git a/commit.h b/commit.h
index b6ad8f3..0f469e5 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
 	const char *output_encoding;
+	int color;
 };
 
 struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..8876c73 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
 	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
+	ctx.color = opt->diffopt.use_color;
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 5bdc2e7..e6a2886 100644
--- a/pretty.c
+++ b/pretty.c
@@ -960,12 +960,19 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 	switch (placeholder[0]) {
 	case 'C':
 		if (placeholder[1] == '(') {
-			const char *end = strchr(placeholder + 2, ')');
+			const char *begin = placeholder + 2;
+			const char *end = strchr(begin, ')');
 			char color[COLOR_MAXLEN];
+
 			if (!end)
 				return 0;
-			color_parse_mem(placeholder + 2,
-					end - (placeholder + 2),
+			if (!memcmp(begin, "auto,", 5)) {
+				if (!want_color(c->pretty_ctx->color))
+					return end - placeholder + 1;
+				begin += 5;
+			}
+			color_parse_mem(begin,
+					end - begin,
 					"--pretty format", color);
 			strbuf_addstr(sb, color);
 			return end - placeholder + 1;
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index c0c62c9..3fc3b74 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -3,6 +3,7 @@ test_description='git rev-list --pretty=format test'
 test_description='git rev-list --pretty=format test'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 test_tick
 test_expect_success 'setup' '
@@ -19,6 +20,18 @@ test_format () {
 	"
 }
 
+# Feed to --format to provide predictable colored sequences.
+AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'
+has_color () {
+	printf '\033[31mfoo\033[m\n' >expect &&
+	test_cmp expect "$1"
+}
+
+has_no_color () {
+	echo foo >expect &&
+	test_cmp expect "$1"
+}
+
 test_format percent %%h <<'EOF'
 commit 131a310eb913d107dd3c09a65d1651175898735d
 %h
@@ -124,6 +137,48 @@ EOF
 ^[[1;31;43mfoo^[[m
 EOF
 
+test_expect_success '%C(auto) does not enable color by default' '
+	git log --format=$AUTO_COLOR -1 >actual &&
+	has_no_color actual
+'
+
+test_expect_success '%C(auto) enables colors for color.diff' '
+	git -c color.diff=always log --format=$AUTO_COLOR -1 >actual &&
+	has_color actual
+'
+
+test_expect_success '%C(auto) enables colors for color.ui' '
+	git -c color.ui=always log --format=$AUTO_COLOR -1 >actual &&
+	has_color actual
+'
+
+test_expect_success '%C(auto) respects --color' '
+	git log --format=$AUTO_COLOR -1 --color >actual &&
+	has_color actual
+'
+
+test_expect_success '%C(auto) respects --no-color' '
+	git -c color.ui=always log --format=$AUTO_COLOR -1 --no-color >actual &&
+	has_no_color actual
+'
+
+test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '
+	(
+		TERM=vt100 && export TERM &&
+		test_terminal \
+			git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+		has_color actual
+	)
+'
+
+test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '
+	(
+		TERM=vt100 && export TERM &&
+		git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+		has_no_color actual
+	)
+'
+
 cat >commit-msg <<'EOF'
 Test printing of complex bodies
 
-- 
1.8.1.rc2.28.gce0611a

Re: [PATCH] log --format: teach %C(auto,black) to paint it black only on terminals

[Jeff King]

On Mon, Dec 17, 2012 at 12:40:55AM -0800, Junio C Hamano wrote:

> +# %C(auto,...) should trump --color=always
> +#
> +# NEEDSWORK: --color=never should also be tested but we need to run a
> +# similar test under pseudo-terminal with test_terminal which is too
> +# much hassle for its worth.
> +
> +test_format advanced-colors-forced \
> +	'%C(auto,red yellow bold)foo%C(auto,reset)' --color=always <<'EOF'
> +commit 131a310eb913d107dd3c09a65d1651175898735d
> +foo
> +commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
> +foo
> +EOF

Hmm. I would have expected this to output colors. In other words, for
"auto" to work just like the config-respecting colorization that is
built into git.

Yes, in this toy example, one could always just drop the "auto" when
using --color=always. But part of the point of this is that I could do:

  git config pretty.fake-oneline "%C(auto,yellow)%h%C(auto,reset) %s"

and have it behave like "--oneline", no matter what the user has in
their color.ui config or --color option on the command line.

I think the patch below (on top of yours) does the right thing by
copying the color option from the rev_info diff options into the
pretty-print context, which is the same place that the graph and
log-tree code look. That means you'll get consistent colorization
(either all or nothing) with:

  git log --graph -p --format='%C(auto,blue)%s%C(auto,reset)'

no matter what your setting of color.diff, color.ui, or --color on the
command line.

It also means that pretty-print defaults to "no colors, do not even
check stdout" when people initialize it to all-zeroes. That's probably a
good thing, as it means any callers of format_commit_message have to
consciously opt into allowing colorization in their format messages.

diff --git a/commit.h b/commit.h
index b6ad8f3..0f469e5 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
 	const char *output_encoding;
+	int color;
 };
 
 struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..8876c73 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
 	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
+	ctx.color = opt->diffopt.use_color;
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index 9e51fec..e6a2886 100644
--- a/pretty.c
+++ b/pretty.c
@@ -967,7 +967,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 			if (!end)
 				return 0;
 			if (!memcmp(begin, "auto,", 5)) {
-				if (!want_color(-1))
+				if (!want_color(c->pretty_ctx->color))
 					return end - placeholder + 1;
 				begin += 5;
 			}