Subject: Re: [PATCH] help: colorize man pages

From: Junio C Hamano <[email-masked]>
Date: Wed, 19 May 2021 15:09:32 +0900

"brian m. carlson" <[email-masked]> writes:

> On 2021-05-19 at 01:08:54, Junio C Hamano wrote:
>> "brian m. carlson" <[email-masked]> 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/[email-masked]/