git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* Man pages have colors? A deep dive into groff
@ 2021-05-15 22:10 Felipe Contreras
  2021-05-17 16:48 ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Contreras @ 2021-05-15 22:10 UTC (permalink / raw)
  To: git

Hi,

While I was doing the investigagion for my GNU_ROFF patch [1], I checked
different versions of all the tools (asciidoc, asciidoctor, docbook, and
groff).

When I tested my own compiled version of groff I noticed something
weird: man pages have colors. This did not happen with the version
shipped by Arch Linux.

I did notice the output generated by docbook stylesheets showed
\m[blue], but no color showed up.

It turns out Arch Linux disables colors, and so does Debian, by
disabling a feature called SGR. This happened about 10 years, and the
rationale was that "it doesn't work correctly".

To enable SGR on these distributions you need to do GROFF_SGR=1, but
that is not documented anywhere.

groff does check for a variable GROFF_NO_SGR, but it's the other way
around: SGR is enabled unless that variable is set.

There's other ways your distribution might be screwing up with groff
(for example Arch Linux converts \' to ', which is not correct), so you
might want to check your shipped configuration in:

  /usr/share/groff/site-tmac/man.local

Unfortunately the colors in man pages leave a lot to be desired.

Here is a simple trick I've been using to show some custom colors:

  man() {
    GROFF_NO_SGR=1 \
    LESS_TERMCAP_md=$'\e[1;31m' \
    LESS_TERMCAP_me=$'\e[0m' \
    LESS_TERMCAP_us=$'\e[1;34m' \
    LESS_TERMCAP_ue=$'\e[0m' \
    LESS_TERMCAP_so=$'\e[1;35m' \
    LESS_TERMCAP_se=$'\e[0m' \
    command man "$@"
  }

Hopefully some of you might find this useful.

Cheers.

[1] https://lore.kernel.org/git/20210515115653.922902-2-felipe.contreras@gmail.com/

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-15 22:10 Man pages have colors? A deep dive into groff Felipe Contreras
@ 2021-05-17 16:48 ` Ævar Arnfjörð Bjarmason
  2021-05-17 19:28   ` Junio C Hamano
  2021-05-18  1:28   ` brian m. carlson
  0 siblings, 2 replies; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-17 16:48 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git


On Sat, May 15 2021, Felipe Contreras wrote:

> Hi,
>
> While I was doing the investigagion for my GNU_ROFF patch [1], I checked
> different versions of all the tools (asciidoc, asciidoctor, docbook, and
> groff).
>
> When I tested my own compiled version of groff I noticed something
> weird: man pages have colors. This did not happen with the version
> shipped by Arch Linux.
>
> I did notice the output generated by docbook stylesheets showed
> \m[blue], but no color showed up.
>
> It turns out Arch Linux disables colors, and so does Debian, by
> disabling a feature called SGR. This happened about 10 years, and the
> rationale was that "it doesn't work correctly".
>
> To enable SGR on these distributions you need to do GROFF_SGR=1, but
> that is not documented anywhere.
>
> groff does check for a variable GROFF_NO_SGR, but it's the other way
> around: SGR is enabled unless that variable is set.
>
> There's other ways your distribution might be screwing up with groff
> (for example Arch Linux converts \' to ', which is not correct), so you
> might want to check your shipped configuration in:
>
>   /usr/share/groff/site-tmac/man.local
>
> Unfortunately the colors in man pages leave a lot to be desired.
>
> Here is a simple trick I've been using to show some custom colors:
>
>   man() {
>     GROFF_NO_SGR=1 \
>     LESS_TERMCAP_md=$'\e[1;31m' \
>     LESS_TERMCAP_me=$'\e[0m' \
>     LESS_TERMCAP_us=$'\e[1;34m' \
>     LESS_TERMCAP_ue=$'\e[0m' \
>     LESS_TERMCAP_so=$'\e[1;35m' \
>     LESS_TERMCAP_se=$'\e[0m' \
>     command man "$@"
>   }
>
> Hopefully some of you might find this useful.
>
> Cheers.
>
> [1] https://lore.kernel.org/git/20210515115653.922902-2-felipe.contreras@gmail.com/

This looks much better.

I wonder a good follow-up (hint, hint! :) would be to have
exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
on color.ui (so we'd do it by default with "auto").

Then e.g. "git help git" would look prettier than "man git".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-17 16:48 ` Ævar Arnfjörð Bjarmason
@ 2021-05-17 19:28   ` Junio C Hamano
  2021-05-17 22:44     ` Felipe Contreras
  2021-05-18  1:28   ` brian m. carlson
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2021-05-17 19:28 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Felipe Contreras, git

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> This looks much better.
>
> I wonder a good follow-up (hint, hint! :) would be to have
> exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
> on color.ui (so we'd do it by default with "auto").
>
> Then e.g. "git help git" would look prettier than "man git".

As long as color.man.ui can be used to override the blanket
color.ui, I think it is a good idea.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-17 19:28   ` Junio C Hamano
@ 2021-05-17 22:44     ` Felipe Contreras
  2021-05-17 22:54       ` Randall S. Becker
  2021-05-18  1:27       ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Felipe Contreras @ 2021-05-17 22:44 UTC (permalink / raw)
  To: Junio C Hamano, Ævar Arnfjörð Bjarmason
  Cc: Felipe Contreras, git

Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > This looks much better.
> >
> > I wonder a good follow-up (hint, hint! :) would be to have
> > exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
> > on color.ui (so we'd do it by default with "auto").
> >
> > Then e.g. "git help git" would look prettier than "man git".
> 
> As long as color.man.ui can be used to override the blanket
> color.ui, I think it is a good idea.

Why not use color.pager?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Man pages have colors? A deep dive into groff
  2021-05-17 22:44     ` Felipe Contreras
@ 2021-05-17 22:54       ` Randall S. Becker
  2021-05-17 23:33         ` Felipe Contreras
  2021-05-18  1:27       ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Randall S. Becker @ 2021-05-17 22:54 UTC (permalink / raw)
  To: 'Felipe Contreras', 'Junio C Hamano',
	'Ævar Arnfjörð Bjarmason'
  Cc: git

On May 17, 2021 6:44 PM, Felipe Contreras wrote:
>Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>> > This looks much better.
>> >
>> > I wonder a good follow-up (hint, hint! :) would be to have
>> > exec_man_man() and exec_man_cmd() in builtin/help.c set this
>> > depending on color.ui (so we'd do it by default with "auto").
>> >
>> > Then e.g. "git help git" would look prettier than "man git".
>>
>> As long as color.man.ui can be used to override the blanket color.ui,
>> I think it is a good idea.
>
>Why not use color.pager?

I think there is a lesson to be learned from git checkout; specifically not to overload semantics. Manual representation is a presentation world unto itself that has should not be blended with programs like less. More than that, being someone who loves automated documentation generation, manual representation has a broader semantic that should not be dismissed. There are probably a whole class of colours that ultimately might be requested - might be me - so I'd rather not blend these into color.pager.

Just my tuppence. 

-Randall


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Man pages have colors? A deep dive into groff
  2021-05-17 22:54       ` Randall S. Becker
@ 2021-05-17 23:33         ` Felipe Contreras
  0 siblings, 0 replies; 15+ messages in thread
From: Felipe Contreras @ 2021-05-17 23:33 UTC (permalink / raw)
  To: Randall S. Becker, 'Felipe Contreras',
	'Junio C Hamano',
	'Ævar Arnfjörð Bjarmason'
  Cc: git

Randall S. Becker wrote:
> On May 17, 2021 6:44 PM, Felipe Contreras wrote:
> >Junio C Hamano wrote:
> >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >>
> >> > This looks much better.
> >> >
> >> > I wonder a good follow-up (hint, hint! :) would be to have
> >> > exec_man_man() and exec_man_cmd() in builtin/help.c set this
> >> > depending on color.ui (so we'd do it by default with "auto").
> >> >
> >> > Then e.g. "git help git" would look prettier than "man git".
> >>
> >> As long as color.man.ui can be used to override the blanket color.ui,
> >> I think it is a good idea.
> >
> >Why not use color.pager?
> 
> I think there is a lesson to be learned from git checkout;
> specifically not to overload semantics.

> Manual representation is a presentation world unto itself that has
> should not be blended with programs like less.

Huh? man is basically a grapper for `groff | less`:

From man(1): "man is the system's manual pager".

You could use it for other purposes, like generating DVI files, but
that's not how git uses it. We it in the normal mode, which uses
the environment variable MANPAGER.

This has absolutely nothing to do with man:

  export GROFF_NO_SGR=1
  export LESS_TERMCAP_md=$'\e[1;31m'
  export LESS_TERMCAP_me=$'\e[m'
  zcat /usr/share/man/man1/git.1.gz | groff -T utf8 -m man | less -R

> More than that, being someone who loves automated
> documentation generation, manual representation has a broader semantic
> that should not be dismissed.

But what does that have to do with `git help`?

> There are probably a whole class of colours that ultimately might be
> requested - might be me - so I'd rather not blend these into
> color.pager.

But it's 100% pager-dependent. Not only do you need a pager for this to
work, but you need specifically the `less` pager.

This:

  MANPAGER=more git -c color.man=always help git

Would not work, because `more` doesn't read use the LESS_TERMCAP variables.

So why call it color.man, if it's not really a man thing?

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-17 22:44     ` Felipe Contreras
  2021-05-17 22:54       ` Randall S. Becker
@ 2021-05-18  1:27       ` Junio C Hamano
  2021-05-18  4:27         ` Felipe Contreras
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2021-05-18  1:27 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Ævar Arnfjörð Bjarmason, git

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> 
>> > This looks much better.
>> >
>> > I wonder a good follow-up (hint, hint! :) would be to have
>> > exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
>> > on color.ui (so we'd do it by default with "auto").
>> >
>> > Then e.g. "git help git" would look prettier than "man git".
>> 
>> As long as color.man.ui can be used to override the blanket
>> color.ui, I think it is a good idea.
>
> Why not use color.pager?

I dug a bit to refresh my memory and it turns out that the reason we
should not do so is because it means something totally different.

color.[ch] defines want_color() that applications like "diff" and
"log" can use to see if the application is configured to paint its
output in colors.

When that layer says for that particular application it should be
decided automatically, then we call into color.c::check_auto_color()
which is the only user of pager_use_color (which is set from the
color.pager configuration variable).  The purpose of that call is to
ask if the pager is capable of colors.

So in short, the color.pager is about "is the pager capable of
colors?"  and the color.ui (and color.<cmd>) is about "does the user
wants output from <cmd> in color?"  We need to use color.help or
something that controls whether the user wants help/man in colors,
and perhaps default it to "auto" like color.ui defaults to, which
then in turn would consult "color.pager".  Tying it directly to
"color.pager" is wrong.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-17 16:48 ` Ævar Arnfjörð Bjarmason
  2021-05-17 19:28   ` Junio C Hamano
@ 2021-05-18  1:28   ` brian m. carlson
  2021-05-18  2:12     ` Junio C Hamano
  2021-05-18  4:31     ` Felipe Contreras
  1 sibling, 2 replies; 15+ messages in thread
From: brian m. carlson @ 2021-05-18  1:28 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Felipe Contreras, git

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

On 2021-05-17 at 16:48:04, Ævar Arnfjörð Bjarmason wrote:
> This looks much better.
> 
> I wonder a good follow-up (hint, hint! :) would be to have
> exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
> on color.ui (so we'd do it by default with "auto").
> 
> Then e.g. "git help git" would look prettier than "man git".

As I mentioned on the patch itself, I'd prefer if Git didn't do this.  I
have my own colors configured and don't want Git to render its man
output differently from what I have.  Even if I didn't, I wouldn't want
Git to change the output of man(1) to be different from what's on the
system.

I should point out that I have my shell configuration set up to use
different colors depending on the capability of the terminal, such as
using a 256-color palette when that's supported and a 16-color palette
when it's not, so there is literally no configuration that Git can
provide here that matches my existing settings.

Additionally, colors tend to pose accessibility problems for a lot of
people.  I have normal color vision, but because I use a transparent
background which renders as grey, the standard terminal red is nearly
illegible for me.  I also know people with colorblindness who have
problems with various colors or any colors at all.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  1:28   ` brian m. carlson
@ 2021-05-18  2:12     ` Junio C Hamano
  2021-05-18  4:35       ` Felipe Contreras
  2021-05-18  4:31     ` Felipe Contreras
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2021-05-18  2:12 UTC (permalink / raw)
  To: brian m. carlson
  Cc: Ævar Arnfjörð Bjarmason, Felipe Contreras, git

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> As I mentioned on the patch itself, I'd prefer if Git didn't do this.  I
> have my own colors configured and don't want Git to render its man
> output differently from what I have.  Even if I didn't, I wouldn't want
> Git to change the output of man(1) to be different from what's on the
> system.
>
> I should point out that I have my shell configuration set up to use
> different colors depending on the capability of the terminal, such as
> using a 256-color palette when that's supported and a 16-color palette
> when it's not, so there is literally no configuration that Git can
> provide here that matches my existing settings.

git -c color.man=false help -m" would let you consume the output in
any way you want, I would presume?

> Additionally, colors tend to pose accessibility problems for a lot of
> people.  I have normal color vision, but because I use a transparent
> background which renders as grey, the standard terminal red is nearly
> illegible for me.  I also know people with colorblindness who have
> problems with various colors or any colors at all.

Yes, accessibility issues are real.  But a bit of configuration to
disable colors would rescue our users.  I work on white-background
with black pixels, and colored diff output that shows lost lines on
the same background with red pixels is hard to read for me, but
thanks to color.diff.<slot> settings, I can customize it to draw in
reverse colors.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  1:27       ` Junio C Hamano
@ 2021-05-18  4:27         ` Felipe Contreras
  2021-05-18  7:16           ` Jeff King
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Contreras @ 2021-05-18  4:27 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Ævar Arnfjörð Bjarmason, git

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> > Why not use color.pager?
> 
> I dug a bit to refresh my memory and it turns out that the reason we
> should not do so is because it means something totally different.
> 
> color.[ch] defines want_color() that applications like "diff" and
> "log" can use to see if the application is configured to paint its
> output in colors.
> 
> When that layer says for that particular application it should be
> decided automatically, then we call into color.c::check_auto_color()
> which is the only user of pager_use_color (which is set from the
> color.pager configuration variable).  The purpose of that call is to
> ask if the pager is capable of colors.

That is not true. check_auto_color() returns 1 when the fd is a tty.

For example advice() would indirectly call check_auto_color() and no
pager is involved.

Inside the help built-in want_color(GIT_COLOR_UNKNOWN) will always
return 1 (unless you do something like `git help git > file`).

pager_use_color is completely ignored in this case.

It's used only when 1) the fd is not a tty, 2) it's stdout, 3) git is
using a pager... Only _then_ it checks for pager_use_color
(color.pager).

So no; the first purpose of check_auto_color() is to find out if the fd
is capable of showing colors, secondary, if we are using a pager, then
check the user configuration.

In the case we are using a pager it's not check if the pager is capable
of colors, the pager could be capable of colors and yet the user would
disable them: it's the check for the user preference.

> So in short, the color.pager is about "is the pager capable of
> colors?"

That's not the case.

Even the documentation says so:

  color.pager::
    A boolean to enable/disable colored output when the pager is in
    use (default is true).

> and the color.ui (and color.<cmd>) is about "does the user
> wants output from <cmd> in color?"

> Tying it directly to "color.pager" is wrong.

Why? color.pager is to enable/disable colored output when a pager is in
use.

The fact that it's man executing the pager and not git makes no
difference to the user.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  1:28   ` brian m. carlson
  2021-05-18  2:12     ` Junio C Hamano
@ 2021-05-18  4:31     ` Felipe Contreras
  1 sibling, 0 replies; 15+ messages in thread
From: Felipe Contreras @ 2021-05-18  4:31 UTC (permalink / raw)
  To: brian m. carlson, Ævar Arnfjörð Bjarmason
  Cc: Felipe Contreras, git

brian m. carlson wrote:
> On 2021-05-17 at 16:48:04, Ævar Arnfjörð Bjarmason wrote:
> > This looks much better.
> > 
> > I wonder a good follow-up (hint, hint! :) would be to have
> > exec_man_man() and exec_man_cmd() in builtin/help.c set this depending
> > on color.ui (so we'd do it by default with "auto").
> > 
> > Then e.g. "git help git" would look prettier than "man git".
> 
> As I mentioned on the patch itself, I'd prefer if Git didn't do this.  I
> have my own colors configured and don't want Git to render its man
> output differently from what I have.

It won't.

> Even if I didn't, I wouldn't want Git to change the output of man(1)
> to be different from what's on the system.

That's a preference others don't share.

> I should point out that I have my shell configuration set up to use
> different colors depending on the capability of the terminal, such as
> using a 256-color palette when that's supported and a 16-color palette
> when it's not, so there is literally no configuration that Git can
> provide here that matches my existing settings.

Once again; your configuration is not going to be overridden.

> Additionally, colors tend to pose accessibility problems for a lot of
> people.  I have normal color vision, but because I use a transparent
> background which renders as grey, the standard terminal red is nearly
> illegible for me.  I also know people with colorblindness who have
> problems with various colors or any colors at all.

Their configuration won't be overridden either.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  2:12     ` Junio C Hamano
@ 2021-05-18  4:35       ` Felipe Contreras
  0 siblings, 0 replies; 15+ messages in thread
From: Felipe Contreras @ 2021-05-18  4:35 UTC (permalink / raw)
  To: Junio C Hamano, brian m. carlson
  Cc: Ævar Arnfjörð Bjarmason, Felipe Contreras, git

Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > As I mentioned on the patch itself, I'd prefer if Git didn't do this.  I
> > have my own colors configured and don't want Git to render its man
> > output differently from what I have.  Even if I didn't, I wouldn't want
> > Git to change the output of man(1) to be different from what's on the
> > system.
> >
> > I should point out that I have my shell configuration set up to use
> > different colors depending on the capability of the terminal, such as
> > using a 256-color palette when that's supported and a 16-color palette
> > when it's not, so there is literally no configuration that Git can
> > provide here that matches my existing settings.
> 
> git -c color.man=false help -m" would let you consume the output in
> any way you want, I would presume?

His configuration won't be overridden. He doesn't need to change
a thing.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  4:27         ` Felipe Contreras
@ 2021-05-18  7:16           ` Jeff King
  2021-05-18 13:21             ` Felipe Contreras
  2021-05-18 14:27             ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Jeff King @ 2021-05-18  7:16 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git

On Mon, May 17, 2021 at 11:27:23PM -0500, Felipe Contreras wrote:

> > So in short, the color.pager is about "is the pager capable of
> > colors?"
> 
> That's not the case.
> 
> Even the documentation says so:
> 
>   color.pager::
>     A boolean to enable/disable colored output when the pager is in
>     use (default is true).

I think that documentation misses the reason you'd want to use it.
Likewise, the commit message introducing it (aa086eb813d) sucks, but the
motivation (from [0]) was:

  When I use a pager that escapes the escape character or highlights the
  content itself the output of git diff without the pager should have
  colors but not with the pager.  For example using git diff with a
  pathspec is quite short most of the time.  For git diff I have to
  enable paging manually and run git diff | $PAGER usually but git log
  uses the pager automatically and should not use colors with it.

For a more concrete example, my pager _does_ understand colors, and I
would not want to set pager.color to "false" (because then "git log",
etc, would not show me any colors). But I don't like the man colors you
are suggesting. I want to be able to turn them off by setting
"color.man" or similar to false, not by disabling color for everything
that is paged.

So color.pager being true is _necessary_ for showing colors in paged
outputs, but by itself is not sufficient. We have other per-context
color options (color.diff, color.branch, and so on).

And so likewise, we would want to avoid turning on colors if the user
has set color.pager=false. Usually this is done automatically because
want_color() checks, which knows if we are using the pager or not. But
if we are going to call out to "man" which will invoke another pager,
that caller would have to check pager_use_color themselves (it's yet
another question of whether "the pager can handle color" applies equally
to the pager that Git will run versus the one that man will run).

-Peff

[0] https://lore.kernel.org/git/E1G6zPH-00062L-Je@moooo.ath.cx/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  7:16           ` Jeff King
@ 2021-05-18 13:21             ` Felipe Contreras
  2021-05-18 14:27             ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Felipe Contreras @ 2021-05-18 13:21 UTC (permalink / raw)
  To: Jeff King, Felipe Contreras
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git

Jeff King wrote:
> On Mon, May 17, 2021 at 11:27:23PM -0500, Felipe Contreras wrote:
> 
> > > So in short, the color.pager is about "is the pager capable of
> > > colors?"
> > 
> > That's not the case.
> > 
> > Even the documentation says so:
> > 
> >   color.pager::
> >     A boolean to enable/disable colored output when the pager is in
> >     use (default is true).
> 
> I think that documentation misses the reason you'd want to use it.
> Likewise, the commit message introducing it (aa086eb813d) sucks,

Well, it was 2006. Many of the best practices of today were not followed
back then.

> but the
> motivation (from [0]) was:
> 
>   When I use a pager that escapes the escape character or highlights the
>   content itself the output of git diff without the pager should have
>   colors but not with the pager.  For example using git diff with a
>   pathspec is quite short most of the time.  For git diff I have to
>   enable paging manually and run git diff | $PAGER usually but git log
>   uses the pager automatically and should not use colors with it.

This is aligned with what I said: the uswer wants to disable colors when
using a pager.

Yes, in this instance it's because the pager doesn't support colors, but
that's not always necessarily so. A person with sight problems may use
less (perfectly capable of colors), but yet not want to exercise that
capability.

It's still a preference.

> For a more concrete example, my pager _does_ understand colors, and I
> would not want to set pager.color to "false" (because then "git log",
> etc, would not show me any colors). But I don't like the man colors you
> are suggesting.

You can change them in your environment.

> I want to be able to turn them off by setting "color.man" or similar
> to false, not by disabling color for everything that is paged.

Sure, for that particular case it does make sense. I'll add that.

> So color.pager being true is _necessary_ for showing colors in paged
> outputs, but by itself is not sufficient. We have other per-context
> color options (color.diff, color.branch, and so on).
> 
> And so likewise, we would want to avoid turning on colors if the user
> has set color.pager=false. Usually this is done automatically because
> want_color() checks, which knows if we are using the pager or not. But
> if we are going to call out to "man" which will invoke another pager,
> that caller would have to check pager_use_color themselves (it's yet
> another question of whether "the pager can handle color" applies equally
> to the pager that Git will run versus the one that man will run).

Yes, but we still need to check pager_use_color.

Except... Maybe a user has GIT_PAGER set to a colorless pager, and
MANPAGER to something fancier, in which case color.pager should be
ignored. But that's probably a corner-case nobody is ever going to hit.

Anyway, I've sent an update version with color.man.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Man pages have colors? A deep dive into groff
  2021-05-18  7:16           ` Jeff King
  2021-05-18 13:21             ` Felipe Contreras
@ 2021-05-18 14:27             ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-05-18 14:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Felipe Contreras, Ævar Arnfjörð Bjarmason, git

Jeff King <peff@peff.net> writes:

> On Mon, May 17, 2021 at 11:27:23PM -0500, Felipe Contreras wrote:
>
>> > So in short, the color.pager is about "is the pager capable of
>> > colors?"
>> 
>> That's not the case.
>> 
>> Even the documentation says so:
>> 
>>   color.pager::
>>     A boolean to enable/disable colored output when the pager is in
>>     use (default is true).
>
> I think that documentation misses the reason you'd want to use it.

Thanks for digging and I agree that this one is quite bad---no
wonder it misled to the "why not color.pager?" question.

The configuration is solely used to disable (because it defaults to
true) colors when pager is in use and the configurations on the
application side (i.e. color.ui and friends) specify "auto".  We
should update the description.

> And so likewise, we would want to avoid turning on colors if the user
> has set color.pager=false. Usually this is done automatically because
> want_color() checks, which knows if we are using the pager or not.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-05-18 14:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15 22:10 Man pages have colors? A deep dive into groff Felipe Contreras
2021-05-17 16:48 ` Ævar Arnfjörð Bjarmason
2021-05-17 19:28   ` Junio C Hamano
2021-05-17 22:44     ` Felipe Contreras
2021-05-17 22:54       ` Randall S. Becker
2021-05-17 23:33         ` Felipe Contreras
2021-05-18  1:27       ` Junio C Hamano
2021-05-18  4:27         ` Felipe Contreras
2021-05-18  7:16           ` Jeff King
2021-05-18 13:21             ` Felipe Contreras
2021-05-18 14:27             ` Junio C Hamano
2021-05-18  1:28   ` brian m. carlson
2021-05-18  2:12     ` Junio C Hamano
2021-05-18  4:35       ` Felipe Contreras
2021-05-18  4:31     ` Felipe Contreras

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).