Re: [RFC PATCH 4/4] color.ui config: add "isatty" setting

From: Jeff King <peff@peff.net>
Date: Jun 1, 2018, 05:30 UTC
Message-ID: <20180601053044.GA8292@sigill.intra.peff.net>
In-Reply-To: <874liofgv6.fsf@evledraar.gmail.com>

On Thu, May 31, 2018 at 09:01:49AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 15 quoted lines

> > Is there some case where a pager can only handle color if _it's_ output
> > is going to a tty, and otherwise not?
> 
> Maybe I'm missing something but how is that something we can deal with?
> We just:
> 
>  1. use isatty() to see if we're attached to a terminal
>  2a. If yes and no pager is invoked (e.g. git status) and "auto" we use colors
>  2b. If yes and we're invoking a pager (e.g. git log) and "auto" we use colors
>      3b. At this point we're writing to the pager so isatty() is false,
>          but we set our own GIT_PAGER_IN_USE and turn on colors if "auto"
> 
> I suppose we can imagine a pager that sometimes emits to a terminal and
> sometimes e.g. opens a browser with the output, and we could ourselves
> somehow detect this...

I was imagining something where we remembered the original isatty() value (from before the pager) and then reacted to that. But no, I don't actually have a use case there. I was trying to think through possible reasons to want this "isatty" version of color.ui.

> As noted in the cover letter I started writing this whole thing before
> understanding some of the subtleties, and now I think this "isatty"
> thing is probably pretty useless, and was wondering if others wanted it.

OK, I agree that it doesn't seem all that useful.

Show 6 quoted lines

> Reasons to take it are:
> 
>  1) To make user intent clearer. I.e. we could just also make it a
>     synonym for color.ui=auto & color.pager=false and those used to
>     isatty semantics skimming the docs would more easily find the right
>     thing.

I'd much prefer just having a documentation patch that uses the word "isatty", if that's something we think a user might search for (which seems plausible to me).

Show 5 quoted lines

>  2) If there are any cases where isatty() is true, but we can detect via
>     other means (e.g. inspecting other env variables) that non-pager
>     output can't handle colors some of the time. Of course if we find
>     such cases isatty() would suck more, but that's presumably what
>     isatty() purists want :)

Yeah, I think we can punt on that until such an "other means" comes along.

-Peff