From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 20 Jun 2016 10:09:04 GMT
Subject: Re: [PATCH 5/5] format-patch: avoid freopen()
Message-ID: <alpine.DEB.2.20.1606201208260.22630@virtualbox>
In-Reply-To: <CAPig+cQqworFpRvd-U9sgnyitQEzy6zAKc_091b9fzjuzsFnpA@mail.gmail.com>

Hi Eric,

On Mon, 20 Jun 2016, Eric Sunshine wrote:

> On Mon, Jun 20, 2016 at 2:26 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Sun, 19 Jun 2016, Eric Sunshine wrote:
> >> On Sat, Jun 18, 2016 at 6:04 AM, Johannes Schindelin
> >> <johannes.schindelin@gmx.de> wrote:
> >> >         if (output_directory) {
> >> > +               rev.diffopt.use_color = 0;
> >>
> >> What is this change about? It doesn't seem to be related to anything
> >> else in the patch.
> >
> > Good point, I completely forgot to mention this is the commit message.
> >
> > 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).
> 
> Would it make sense to do this as a separate preparatory patch, or is
> it just too minor?

That's a good point. It *is* a change in its own right. Will reroll.

Thanks,
Dscho