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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2016, 15:49 UTC
Message-ID: <xmqqeg7pns0a.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1606220849480.10382@virtualbox>

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

> But there's a rub... If you specify --color *explicitly*, use_color is set
> to GIT_COLOR_ALWAYS and the file indeed contains ANSI sequences (i.e. my
> analysis above left out the command-line part).

Heh, the command-line is the _ONLY_ thing I raised, as we knew ui.color is not an issue in this codepath, in $gmane/297757.

Going back to that and reading again, I suggested to check with GIT_COLOR_AUTO (i.e. if it is left to "auto", disable it) because I think the former is a much more future-proof way (imagine that we may add --color=<some new setting> in the future) than checking with GIT_COLOR_ALWAYS (i.e. if it is not explicitly set to "always", disable it).

> In short, I think you're right, I have to guard the assignment, with the
> minor adjustment to test use_color != GIT_COLOR_ALWAYS.

So I am not sure if you said "use_color != GIT_COLOR_ALWAYS" only to be different from what I suggested (you seem to have a tendency to do so whenever you can), or there was some other reason.