Re: [PATCH v4 06/10] format-patch: explicitly switch off color when writing to files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Fri, 24 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> 
> > We rely on the auto-detection ("is stdout a terminal?") to determine
> > whether to use color in the output of format-patch or not. That
> > happens to work because we freopen() stdout when redirecting the
> > output to files.
> >
> > However, we are about to fix that work-around, in which case the
> > auto-detection has no chance to guess whether to use color or not.
> >
> > But then, we do not need to guess to begin with. As argued in the
> > commit message of 7787570c (format-patch: ignore ui.color,
> > 2011-09-13), we do not allow the ui.color setting to affect
> > format-patch's output. The only time, therefore, that we allow color
> > sequences to be written to the output files is when the user specified
> > the --color command-line option explicitly.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> 
> The right fix in the longer term (long after this series lands, that
> is) is probably to update the world view that the codepath from
> want_color_auto() to check_auto_color() has always held.  In their
> world view, when they are asked to make --color=auto decision, the
> output always goes the standard output, and that is why they
> hardcode isatty(1) to decide.  The existing freopen() was a part of
> that world view.

I agree, and I briefly looked into this. It looks a bit more involved than
I am prepared for, what with my real goal being the rebase--helper, not
clean-ups ;-)

> We'd need a workaround like this patch if we want to leave the
> want_color_auto() as-is, and as a workaround I think this is the
> least invasive one, so let's queue it as-is.

Thanks!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]