Re: [PATCH 5/5] format-patch: avoid freopen()
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jun 20, 2016, 10:09 UTC
- 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:
Show 26 quoted lines
> 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