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

From: Jeff King <peff@peff.net>
Date: Dec 17, 2012, 12:13 UTC
Message-ID: <20121217121354.GB21858@sigill.intra.peff.net>
In-Reply-To: <CACsJy8Dt4vEpO+EcAhWnko=XAajQ9OMgbDbVx78Eb=sZTjmKQA@mail.gmail.com>

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

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

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