Re: [PATCH v4] help: colorize man pages

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 20, 2021, 13:58 UTC
Message-ID: <60a66b11d6ffd_2448320885@natae.notmuch>
In-Reply-To: <842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com>

Phillip Wood wrote:

Show 6 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

It makes a difference for git developers, not for the user.

The user doesn't care how the output of `git grep` was generated, all she sees is that it's different from `grep`. It's in fact more surprising than a difference in `git help` because it's even the same comand.

Maybe if the command was `git man` they would be equally surprising, but it's not, in fact, `git help` can be used to 1) output directly to the terminal 2) view in a browser, 3) view in info program, 4) view man page in woman, 5) view the man page in koqueror 6) view the man page in man.

Only in one case among many would the user expect to see man, therefore a colorized `git grep` is more surprising.

Show 18 quoted lines

> > Our man pages don't contain many useful colors (just blue links),
> > moreover, many people have groff SGR disabled, so they don't see any
> > colors with man pages.
> > 
> > We can set the LESS variable to render bold, underlined, and standout
> > text with colors in the less pager.
> > 
> > Bold is rendered as red, underlined as blue, and standout (prompt and
> > highlighted search) as inverse magenta.
> > 
> > Obviously this only works when the less pager is used.
> > 
> > If the user has already set the LESS variable in his/her environment,
> > that is respected, and nothing changes.
> 
> However if they have specified the colors they would like by using the 
> LESS_TERMCAP_xx environment variables that the previous versions of this 
> patch used their choice is overridden by this new patch.

That is true. We could add a check for that:

  if (getenv("LESS_TERMCAP_md"))
          return;

However, it may not be necessary since many of the tips online set these variables inside a function.

> I've got LESS_TERMCAP_xx set and running
> 	LESS='Dd+r$Du+b$Ds' man git add
> changes the output colors

You have them set in the environtment? Not inside a function like man () { ... command man "$@" } ?

Show 8 quoted lines

> > A new color configuration is added: `color.man` for the people that want
> > to turn this feature off, otherwise `color.ui` is respected.
> > Additionally, if color.pager is not enabled, this is disregarded.
> > 
> > Normally check_auto_color() would check the value of `color.pager`, but
> > in this particular case it's not git the one executing the pager, but
> 
> s/git the one/git is not/

You mean s/it's not git/git is not/

Fine by me.

Cheers.

-- 
Felipe Contreras