Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

5 messages, 4 authors, 2021-06-24 · open the first message on its own page

Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

From: Junio C Hamano <hidden>
Date: 2021-06-21 10:28:56

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
So, in order for this change to have any effect:

 1. color.man=true must be set in the config
 2. The user must use less
 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)
 4. Have color.ui enabled
 5. Not have color.pager disabled
 6. Not have git with stdout directed to a file

1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/
2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147

Suggested-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Felipe Contreras <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

On Tue, Jun 08 2021, Junio C Hamano wrote:
quoted
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
I've been running with this on my personal git build since May 26th. I
haven't had any issues with it, and I like the new coloring.
...
I think this is a good example of a change that we're better off just
merging down and then reverting if the wider audience of git users hates
it, rather than trying to come to some perfect consensus here
on-list.
My impression was tht we already had a rough consensus here on-list
that it may be good to educate users who like this "new coloring"
like you do to configure their "less", so that they consistently get
the "new coloring" they like whether they are doing "git help git",
"man git", or even "man ls", and the approach the posted patch takes
will not help (it only affects "git help git" among these).

I'd rather not to take it.
Fair enough, here's a version I think you and others will find
acceptable then. It allows users like me who like this to explicitly
opt-in via color.man=true.
Not really.

Since the implementation of the posted patch, as I understand it,
does not aim to affect both "git help -m foo" and "man git-foo"
identically, I think it would be easier to understand to end-users
if this were exposed as a new "mode", like "git help --web" and "git
help --info" are different modes from the "git help --man",
something like "git help --fancy-man" (or whatever is easy to type
and explain, and also add it to the variants help.format knows about
to make it easy to set the default).

One advantage of doing so is that we do not have to worry about "ah,
user has LESS_BLAH environment variable so we should disable this
new mode here" etc.  As long as the new mode is requested either via
the command line option or help.format configuration, it can
completely take it over.  That simplifies the necessary explanation
given to the users quite a lot, no?
	----
	git-config - Get and set [...]

	SYNOPSIS
	--------
	[...]
	'git config' [<file-option>] [...]
	[...]
	The `--type=<type>` option instructs 'git config' to ensure [...]

Will have "NAME" and "SECTION" shown as BOLD RED instead of BOLD, "git
config" and other '-quoted parts in BLUE UNDERLINE instead of
UNDERLINE, and `--type=<type>` and other `-quoted parts in RED BOLD
instead of BOLD. The "Standout" setting is then used for the user's
own search bar (invoked with "/") and prompt. See [2] for more
examples
There are BOLD RED and READ BOLD; are they differently rendered?

Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

From: Felipe Contreras <hidden>
Date: 2021-06-21 18:41:42

Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted
Fair enough, here's a version I think you and others will find
acceptable then. It allows users like me who like this to explicitly
opt-in via color.man=true.
Not really.

Since the implementation of the posted patch, as I understand it,
does not aim to affect both "git help -m foo" and "man git-foo"
identically,
It cannot aim to do what is not possible.
I think it would be easier to understand to end-users
if this were exposed as a new "mode", like "git help --web" and "git
help --info" are different modes from the "git help --man",
something like "git help --fancy-man" (or whatever is easy to type
and explain, and also add it to the variants help.format knows about
to make it easy to set the default).
But it is not a new mode.

I presume you mean format, since man, info, and web are formats,
controlled by help.format.

But no, it's not a format either, because we still want to see the same
format (man).

Perhaps you meant a man viewer (controlled with man.viewer), but there's
no command line option to launch help with for example emacs woman.

But still, it's not a new viewer; it's an improvement of an already
existing viewer.
One advantage of doing so is that we do not have to worry about "ah,
user has LESS_BLAH environment variable so we should disable this
new mode here" etc.
We don't have to worry about that with the current patch.
As long as the new mode is requested either via
the command line option or help.format configuration, it can
completely take it over.
That is already the case.
That simplifies the necessary explanation given to the users quite a
lot, no?
No, it would still be:

 1. man.viewer=fancyman must be set in the config
 2. The user must use less
 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)
 4. Have color.ui enabled
 5. Not have color.pager disabled
 6. Not have git with stdout directed to a file

Moreover, this explanation is for developers.

Realistically all the user needs to know is that color.man=true turns
this on (man.viewer=fancyman is not better in any way).

-- 
Felipe Contreras

Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-06-21 19:23:01

On Mon, Jun 21 2021, Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted
So, in order for this change to have any effect:

 1. color.man=true must be set in the config
 2. The user must use less
 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)
 4. Have color.ui enabled
 5. Not have color.pager disabled
 6. Not have git with stdout directed to a file

1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/
2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147

Suggested-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Felipe Contreras <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

On Tue, Jun 08 2021, Junio C Hamano wrote:
quoted
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
I've been running with this on my personal git build since May 26th. I
haven't had any issues with it, and I like the new coloring.
...
I think this is a good example of a change that we're better off just
merging down and then reverting if the wider audience of git users hates
it, rather than trying to come to some perfect consensus here
on-list.
My impression was tht we already had a rough consensus here on-list
that it may be good to educate users who like this "new coloring"
like you do to configure their "less", so that they consistently get
the "new coloring" they like whether they are doing "git help git",
"man git", or even "man ls", and the approach the posted patch takes
will not help (it only affects "git help git" among these).

I'd rather not to take it.
Fair enough, here's a version I think you and others will find
acceptable then. It allows users like me who like this to explicitly
opt-in via color.man=true.
Not really.

[snip] I think it would be easier to understand to end-users
if this were exposed as a new "mode", like "git help --web" and "git
help --info" are different modes from the "git help --man",
something like "git help --fancy-man" (or whatever is easy to type
and explain, and also add it to the variants help.format knows about
to make it easy to set the default).

One advantage of doing so is that we do not have to worry about "ah,
user has LESS_BLAH environment variable so we should disable this
new mode here" etc.  As long as the new mode is requested either via
the command line option or help.format configuration, it can
completely take it over.  That simplifies the necessary explanation
given to the users quite a lot, no?
The interaction between "git help" and "man"/"less" doesn't really have
an equivalent in the rest of git as far as color output goes. Usually we
emit colors via our own programs.

But no, I think it makes the most sense to consider this orthagonal to
help.format=man or man.viewer=<cmd>.

We're not invoking a different man viewer or command, we're just
expecting that mode to invoke the pager, and if that pager is less to
have these variables tweak our color preferences.
[unsnip] Since the implementation of the posted patch, as I understand it,
does not aim to affect both "git help -m foo" and "man git-foo"
identically, 
Aside from this patch I don't think it makes sense to view git's UI and
interaction with the pager like that.

To probably >95% of our users the "we invoke the pager" is just a
technical implementation details they're not aware of. Git just has
pretty colors by default, so I don't think it's jarring that "git help
xyz" and "man git-xyz" look different.

We also don't try to maintain the UI that:

    git cmd >file &&
    pager file

Gives you the same UX as:

    # Invokes pager(1) for you
    git cmd

Since we set e.g. PAGER_ENV already, I believe this was brought up in
past discussions. So we're already past the point of git adding its own
magic custom options to feed to the pager.

So I don't see the problem with having "a bit more like PAGER_ENV"
hidden behind a color.man=true config option in this case.
quoted
	----
	git-config - Get and set [...]

	SYNOPSIS
	--------
	[...]
	'git config' [<file-option>] [...]
	[...]
	The `--type=<type>` option instructs 'git config' to ensure [...]

Will have "NAME" and "SECTION" shown as BOLD RED instead of BOLD, "git
config" and other '-quoted parts in BLUE UNDERLINE instead of
UNDERLINE, and `--type=<type>` and other `-quoted parts in RED BOLD
instead of BOLD. The "Standout" setting is then used for the user's
own search bar (invoked with "/") and prompt. See [2] for more
examples
There are BOLD RED and READ BOLD; are they differently rendered?
Yes, that's just an omission/mistake. It should be the "RED BOLD",
i.e. "<COLOR> <ATTR>".

Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

From: Jeff King <hidden>
Date: 2021-06-23 23:58:58

On Mon, Jun 21, 2021 at 09:08:20PM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted
[snip] I think it would be easier to understand to end-users
if this were exposed as a new "mode", like "git help --web" and "git
help --info" are different modes from the "git help --man",
something like "git help --fancy-man" (or whatever is easy to type
and explain, and also add it to the variants help.format knows about
to make it easy to set the default).

One advantage of doing so is that we do not have to worry about "ah,
user has LESS_BLAH environment variable so we should disable this
new mode here" etc.  As long as the new mode is requested either via
the command line option or help.format configuration, it can
completely take it over.  That simplifies the necessary explanation
given to the users quite a lot, no?
The interaction between "git help" and "man"/"less" doesn't really have
an equivalent in the rest of git as far as color output goes. Usually we
emit colors via our own programs.

But no, I think it makes the most sense to consider this orthagonal to
help.format=man or man.viewer=<cmd>.

We're not invoking a different man viewer or command, we're just
expecting that mode to invoke the pager, and if that pager is less to
have these variables tweak our color preferences.
FWIW, if we are going to do this, then just having it as "color.man"
makes the most sense to me. It is easily explained as "when we invoke
man, set up some environment variables that may enable colors in the
output".

I'm still entirely unconvinced that this should be in Git at all;
pointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like
it would be sufficient. But it feels like that conversation was not
going anywhere productive; I mention it here only to indicate that my
response above is not an endorsement of the concept.

-Peff

Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)

From: Felipe Contreras <hidden>
Date: 2021-06-24 01:03:16

Jeff King wrote:
On Mon, Jun 21, 2021 at 09:08:20PM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted
quoted
[snip] I think it would be easier to understand to end-users
if this were exposed as a new "mode", like "git help --web" and "git
help --info" are different modes from the "git help --man",
something like "git help --fancy-man" (or whatever is easy to type
and explain, and also add it to the variants help.format knows about
to make it easy to set the default).

One advantage of doing so is that we do not have to worry about "ah,
user has LESS_BLAH environment variable so we should disable this
new mode here" etc.  As long as the new mode is requested either via
the command line option or help.format configuration, it can
completely take it over.  That simplifies the necessary explanation
given to the users quite a lot, no?
The interaction between "git help" and "man"/"less" doesn't really have
an equivalent in the rest of git as far as color output goes. Usually we
emit colors via our own programs.

But no, I think it makes the most sense to consider this orthagonal to
help.format=man or man.viewer=<cmd>.

We're not invoking a different man viewer or command, we're just
expecting that mode to invoke the pager, and if that pager is less to
have these variables tweak our color preferences.
FWIW, if we are going to do this, then just having it as "color.man"
makes the most sense to me. It is easily explained as "when we invoke
man, set up some environment variables that may enable colors in the
output".

I'm still entirely unconvinced that this should be in Git at all;
That's OK, you don't need to be convinced for this change to be a
positive one.
pointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like
it would be sufficient.
What color-man wrapper?
But it feels like that conversation was not going anywhere productive;
Feelings are not facts.

Bailing from a discussion doesn't resolve the discussion, and the
question "how is a user supposed to configure this properly?" remains
unanswered by you, or anyone [1].

This patch is the closest to a convenient solution anybody has come up
with.

If anybody has any other proposal it would be good to hear them.

[1] https://lore.kernel.org/git/60bfadc0aca09_1abb8f208fd@natae.notmuch/

-- 
Felipe Contreras
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help