Re: [PATCH] help: colorize man pages

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

 



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

> "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes:
>
>> On 2021-05-19 at 01:08:54, Junio C Hamano wrote:
>>> "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxx/

[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.



[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]

  Powered by Linux