Re: [PATCH 0/1] Colorize some errors on stderr

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 6, 2018, 11:21 UTC
Message-ID: <nycvar.QRO.7.76.6.1804061315500.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <xmqqwozc68r2.fsf@gitster-ct.c.googlers.com>

Hi Junio,

[and welcome back, at least I hope you only read this after a good and relaxing vacation]

On Fri, 16 Feb 2018, Junio C Hamano wrote:

Show 19 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Now, what would be possible solutions for this?
> >
> > - introduce `int fd` in `want_color()` (and callees) so that we can make
> >   a distinction whether we want to detect whether stdout or stderr is connected
> >   to a tty
> >
> > - introduce a separate `want_color_stderr()` (we still would need to decide
> >   whether we want a config setting for this)
> 
> Between the above two, there probably aren't so big a difference, but
> in order to avoid disrupting existing callers of want_color() while
> possibly sharing as much code between the old and new callers,
> perhaps:
> 
> 	extern int want_color_fd(int fd, int colorbool);
> 	#define want_color(colorbool) want_color_fd(1, (colorbool))
> 	#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))

I made it so.

Note that I also had to change the check_auto_color() function, and while want_color_fd() can have a "private" record of previous results, check_auto_color() needs to use the global color_stdout_is_tty (so that builtin/config.c can edit it, for use in `git config --colorbool <name> [stdout-is-tty]`).

> We should honor configuration at two levels, just like the colors on
> stdout, i.e. color in which individual items are painted (e.g.
> color.diff.filename, color.advice.hint) and whether we use colors in
> UI at all (e.g. color.ui).

This is how v2 does it.

Thanks for your suggestions, Dscho