git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	git@vger.kernel.org, "Randall S. Becker" <rsbecker@nexbridge.com>,
	Leah Neukirchen <leah@vuxu.org>
Subject: Re: [PATCH] help: colorize man pages
Date: Wed, 19 May 2021 10:41:44 +0200	[thread overview]
Message-ID: <87lf8bqdv0.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqq1ra3z23n.fsf@gitster.g>


On Wed, May 19 2021, Junio C Hamano wrote:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> On 2021-05-19 at 01:08:54, Junio C Hamano wrote:
>>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>>> 
>>> > In general, this is made worse because Git doesn't honor the unofficial
>>> > but widely supported NO_COLOR[0], so reading the documentation is
>>> > obligatory.
>>> 
>>> I vaguely recall that we were contacted by NO_COLOR folks to be
>>> an early supporter of their cause to break the chicken-and-egg
>>> problem they were hagving, and (unhelpfully) answered with "sure,
>>> when we see enough people support it---otherwise we'd end up having
>>> to keep essentially a dead code that supports a convention that is
>>> not all that useful".
>>
>> Yeah, I seem to recall you were somewhat negative on it at the time, but
>> I do personally find it useful, and someone on Twitter reminded me of
>> it just today.
>>
>>> I wonderr if it is just a matter of hooking into want_color(), like this?
>>> 
>>>  color.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>> 
>>> diff --git c/color.c w/color.c
>>> index 64f52a4f93..2516ef7275 100644
>>> --- c/color.c
>>> +++ w/color.c
>>> @@ -373,12 +373,17 @@ int want_color_fd(int fd, int var)
>>>  	 * we always write the same value, but it's still wrong. This function
>>>  	 * is listed in .tsan-suppressions for the time being.
>>>  	 */
>>> -
>>> +	static int no_color = -1;
>>>  	static int want_auto[3] = { -1, -1, -1 };
>>>  
>>>  	if (fd < 1 || fd >= ARRAY_SIZE(want_auto))
>>>  		BUG("file descriptor out of range: %d", fd);
>>>  
>>> +	if (no_color < 0)
>>> +		no_color = !!getenv("NO_COLOR");
>>> +	if (no_color)
>>> +		return 0;
>>> +
>>>  	if (var < 0)
>>>  		var = git_use_color_default;
>>>  
>>
>> Yeah, that will probably do it.  I hadn't looked at it, but I assumed it
>> would be pretty easy, and it looks like it is.
>
> Actually I doubt it satisfies the FAQ #2 of no-color.org; we
> probably would need to go one level lower, like the original
> proposal from 2018 did:
>
> cf. https://lore.kernel.org/git/87efl3emlm.fsf@vuxu.org/

[CC'd the author of that proposal]

It also doesn't seem to me to satisfy their FAQ point #1, i.e. users who
actually want no color at all can just set TERM=dumb, and we support
that. The proposed patch is the same as having TERM=dumb set.

This NO_COLOR=1 actually means something like "I do support colors, so
show them if it's important, but don't color things willy-nilly".

I'm not sure if it matters for git, the FAQ point isn't really clear on
what the distinction is exactly. Users who want to use color for say CLI
emacs/vim/screen/tmux "status" bars, but don't want any "normal" CLI
program to emit them?

But if we gained such a "status" bar feature the proposed 2018 patch
would be actively going against what NO_COLOR users want, since it's our
equivalent of TERM=dumb, not whatever NO_COLOR=1 is supposed to mean. Or
maybe we already have that, I would think that "git add -i"'s UI would
count.

It seems like it really should have been named MOSTLY_NOT_COLOR=1 or
ONLY_COLOR_NCURSES_LIKE_UIS=1 if I'm understanding that FAQ item
correctly.

So it would be incorrect to map it to either color.ui=never or
color.ui=always (as "auto" will implicitly do). We'd need a new knob to
control the granularity of coloring, something like
color.ui=conservative.

I wasn't against NO_COLOR before, but after writing the above I think I
am. I initially assumed that it was some redundant and more "friendly"
way of setting TERM=dumb, but rather it's some entirely subjective way
for every program to decide if their UI elements are "text-editor"-like
or "status bar"-like enough to warrant coloring.

That's "against" in the sense that if git supported it I wouldn't care
much, and wouldn't oppose a patch to implement it.

But it seems to me to just introduce even more confusion to the *nix
coloring landscape. For what it's apparently trying to accomplish I
think it would be a much better thing to:

 1. Have terminals/startup rc'd etc. set a TERM_ACTUAL=<old value>
    before setting TERM=dumb. This is something POSIX et al could
    eventually standardize, i.e. "TERM=dumb" for now, but actually I
    support "TERM=xyz".

 2. Have some "color_this" shell function/alias/wrapper to start things
    like your editor, which would just be a one-line wrapper to start
    that program with TERM=$TERM_ACTUAL, or those programs would learn
    to look at TERM_ACTUAL.

The user would thus get color almost nowhere in "normal" programs like
"git status" or "ls", but would get them in emacs, vim, screet, tmux,
htop or whatever other "big" terminal UI they run.

I.e. the whole point seems to be to support the use-case of wanting
color almost nowhere except a very small whitelist of programs, but
trying to accomplish it with NO_COLOR means that hundreds/thousands of
programs need to support it, as opposed to the much smaller list of
editors/terminal multiplexers etc.

Each of those programs then need to subjectively decide if their UI
elements are "such as [...] a status bar". If they get it wrong the user
is back to inovking them with TERM=dumb anyway.

  reply	other threads:[~2021-05-19  9:20 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18  1:01 Felipe Contreras
2021-05-18  1:19 ` brian m. carlson
2021-05-18  3:22   ` Felipe Contreras
2021-05-18 23:49     ` brian m. carlson
2021-05-19  1:08       ` Junio C Hamano
2021-05-19  2:07         ` brian m. carlson
2021-05-19  6:09           ` Junio C Hamano
2021-05-19  8:41             ` Ævar Arnfjörð Bjarmason [this message]
2021-05-19 10:36               ` Felipe Contreras
2021-05-21  0:58               ` brian m. carlson
2021-05-21 18:09                 ` Felipe Contreras
2021-05-21 19:48                   ` Igor Djordjevic
2021-05-21 21:20                     ` Felipe Contreras
2021-05-21 22:10                       ` Igor Djordjevic
2021-05-21 23:04                         ` Felipe Contreras
2021-05-22 18:38                           ` Igor Djordjevic
2021-05-22 21:48                             ` Felipe Contreras
2021-05-23 11:25                               ` Igor Djordjevic
2021-05-23 14:48                                 ` Felipe Contreras
2021-05-21 22:47                     ` Igor Djordjevic
2021-05-21 23:32                     ` Junio C Hamano
2021-05-19  9:26       ` Ævar Arnfjörð Bjarmason
2021-05-19 10:10         ` Jeff King
2021-05-19 11:45           ` Felipe Contreras
2021-05-19 11:19         ` Felipe Contreras
2021-05-19 12:21           ` Felipe Contreras
2021-05-20  1:55         ` brian m. carlson
2021-05-20  2:23           ` Junio C Hamano
2021-05-20  3:05             ` Felipe Contreras
2021-05-20  3:28               ` Junio C Hamano
2021-05-20  3:48                 ` Felipe Contreras
2021-05-20  2:45           ` Felipe Contreras
2021-05-19 10:25       ` Felipe Contreras

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

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

  git send-email \
    --in-reply-to=87lf8bqdv0.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=leah@vuxu.org \
    --cc=rsbecker@nexbridge.com \
    --cc=sandals@crustytoothpaste.net \
    --subject='Re: [PATCH] help: colorize man pages' \
    /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

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 NNTP newsgroup(s).