Re: [PATCH v4] help: colorize man pages

From: Junio C Hamano <gitster@pobox.com>
Date: May 21, 2021, 05:06 UTC
Message-ID: <xmqqbl94smjb.fsf@gitster.g>
In-Reply-To: <842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 8 quoted lines

> On 20/05/2021 05:07, Felipe Contreras wrote:
>> We already colorize tools traditionally not colorized by default, like
>> diff and grep. Let's do the same for man.
>
> I think there is a distinction between 'diff' and 'grep' where we are
> generating the content and help where we are running man - I would 
> expect a man page to look the same whether it is displayed by 'man git
> foo' or 'git help foo'

... as long as the user chooses "man" backend, that is. And I tend to agree, but that is our expectation.

If we added this new mode of driving the same "man" but with different environment variables exported to tweak how "less" behaves, and taught it to builtin/help.c::exec_viewer() and builtin/help.c::man_viewer_list, that might become more palatable in the sense that we can view it as feeding the same manual page to this another "man" that behaves differently from the plain "man", just like we can feed it to "woman" or "konqueror" to get a different view. So those (like you and I) who expect a man page to look the same in "man git foo" and "git help -m foo" can keep using our current configuration, while those who want yet another variant of "man" output in addition to the current "man", "woman", and "konqueror" can choose it and get "colorized" output.

By the way, this new round mentions NO_COLOR, and while I think it is good idea to teach git to honor it, I think it does it at a wrong level. Each ui driver that is optionally capable of coloring its output shouldn't have to care, and the right level is either inside want_color() or its helper function check_auto_color(), both in color.c, to say "the user hasn't configured the output of this subcommand for coloring, and by default we use color under certain conditions (i.e. "auto"), but we decide not to use color because NO_COLOR environment is set before even checking these "auto" conditions.