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

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/ (local)

-- 
Felipe Contreras