Re: [PATCH 5/5] format-patch: avoid freopen()

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 20, 2016, 16:03 UTC
Message-ID: <xmqq4m8nvodo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1606200814510.22630@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 10 quoted lines

> When format-patch calls the diff machinery, want_color() is used to
> determine whether to use ANSI color sequences or not. If use_color is not
> set explicitly, isatty(1) is used to determine whether or not the user
> wants color. When stdout was freopen()ed, this isatty(1) call naturally
> looked at the file descriptor that was reopened, and determined correctly
> that no color was desired.
>
> With the freopen() call gone, stdout may very well be the terminal. But we
> still do not want color because the output is intended to go to a file (at
> least if output_directory is set).

How does this interact with --color=... that is given from the command line at the same time? Currently --color=always would give you a coloured output.

I personally do not think of a sensible reason why anybody wants a colored format-patch output, but Git's userbase has grown large enough and you can no longer expect that only sensible people use it ;-)

You can probably sell "when giving out put to file, we will never color the output" as an improved new world order, but if that is what this change wants to do, it probably deserves a separate patch.

I however think you can avoid breaking expectations by people who are not so sensible by overriding only when use_color is set to GIT_COLOR_AUTO, perhaps?