git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Stefan Beller <sbeller@google.com>
Subject: Re: [RFC PATCH 4/4] color.ui config: add "isatty" setting
Date: Thu, 31 May 2018 09:01:49 +0200	[thread overview]
Message-ID: <874liofgv6.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20180531053810.GD17068@sigill.intra.peff.net>


On Thu, May 31 2018, Jeff King wrote:

> On Wed, May 30, 2018 at 09:06:41PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> A co-worker of mine who was using UNIX systems when dinosaurs roamed
>> the earth was lamenting that kids these days were using tools like
>> "git" that thought they knew better than isatty(3) when deciding
>> whether or not something was a terminal, and the state of the
>> documentation fixed earlier in this series certainly didn't help.
>>
>> So this setting is a small gift to all the UNIX graybeards out
>> there. Now they can set color.ui=isatty and only emit fancy colors in
>> situations when the gods of old intended, not whatever heuristic we've
>> decided to set "auto" to.
>
> I'm having trouble thinking of a case where anybody would actually
> care about the distinction between these two.
>
> If you're not using "-p", then colors will kick in if isatty() is true.
> The whole "also check the pager" thing is only because we may check the
> color isatty() after starting the pager. If we didn't, then nobody would
> ever see color. If your pager doesn't understand color, then fine. Tell
> Git not to send it color with color.pager=false.
>
> If you are using "-p", we might send color to your pager even though you
> weren't originally going to a terminal. But either your pager can handle
> colors, in which case this is fine, or it cannot, in which case you
> would already have needed to set color.pager as above to un-break all of
> the non-p cases.
>
> 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...

> And you'd want color predicated on
> whether the original output was a tty or not, even if you said "-p"?
> That's the only case I can think of where the existing config is not
> sufficient, but I'm having a hard time envisioning a practical use.
>
>> As noted here this is *currently* the same as setting color.ui=auto &
>> color.pager=false, but I think it's good to explicitly have this
>> setting for any future changes. The reason, as now noted in the
>> documentation is that the "auto" setting may become even smarter in
>> the future and learn even deeper heuristics for when to turn itself on
>> even if isatty(3) were returning true.
>
> Are we actually thinking about teaching it deeper heuristics? I think
> the fact that it _is_ based on isatty() is meant to be understood by the
> user, and we wouldn't want to change that. True, there is the pager
> exception, but the point of that is to maintain the "are we going to a
> tty" model, even in the face of sticking a pager in the middle.

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.

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.

 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 :)


  reply	other threads:[~2018-05-31  7:05 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30 21:06 [PATCH 0/4] color.ui docs & add color.ui=isatty Ævar Arnfjörð Bjarmason
2018-05-30 21:06 ` [PATCH 1/4] config doc: move color.ui documentation to one place Ævar Arnfjörð Bjarmason
2018-05-31  5:25   ` Jeff King
2018-05-31  7:09     ` Ævar Arnfjörð Bjarmason
2018-06-01  5:31       ` Jeff King
2018-05-30 21:06 ` [PATCH 2/4] config doc: clarify "to a terminal" in color.ui Ævar Arnfjörð Bjarmason
2018-05-31  5:27   ` Jeff King
2018-05-30 21:06 ` [RFC PATCH 3/4] color.ui config: don't die on unknown values Ævar Arnfjörð Bjarmason
2018-05-30 22:32   ` Stefan Beller
2018-05-30 23:05   ` Junio C Hamano
2018-05-31  7:17     ` Ævar Arnfjörð Bjarmason
2018-06-01  5:53       ` Jeff King
2018-05-30 21:06 ` [RFC PATCH 4/4] color.ui config: add "isatty" setting Ævar Arnfjörð Bjarmason
2018-05-30 22:57   ` Junio C Hamano
2018-05-31  7:07     ` Ævar Arnfjörð Bjarmason
2018-05-31  5:38   ` Jeff King
2018-05-31  7:01     ` Ævar Arnfjörð Bjarmason [this message]
2018-06-01  5:30       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874liofgv6.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).