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

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Dec 17, 2012, 11:44 UTC
Message-ID: <CACsJy8Dt4vEpO+EcAhWnko=XAajQ9OMgbDbVx78Eb=sZTjmKQA@mail.gmail.com>
In-Reply-To: <7vobhtm5pk.fsf_-_@alter.siamese.dyndns.org>

On Mon, Dec 17, 2012 at 3:40 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> 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.

Show 7 quoted lines

>                         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