diff mbox series

[v6] help: colorize man pages

Message ID 20210523054454.1188757-1-felipe.contreras@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v6] help: colorize man pages | expand

Commit Message

Felipe Contreras May 23, 2021, 5:44 a.m. UTC
We already colorize tools traditionally not colorized by default, like

Comments

Phillip Wood May 24, 2021, 1:13 p.m. UTC | #1
On 23/05/2021 06:44, Felipe Contreras wrote:
> We already colorize tools traditionally not colorized by default, like
> diff and grep. Let's do the same for man.
> 
> 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 LESS_TERMCAP variables to render bold and underlined text
> with colors in the pager; a common trick[1].
> 
> Bold is rendered as red, underlined as blue, and standout (prompt and
> highlighted search) as inverse cyan.
> 
> Obviously this only works when the less pager is used.
> 
> If the user already has LESS_TERMCAP variables set in his/her
> environment, those are respected and nothing changes.
> 
> 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
> man. Therefore we need to check pager_use_color ourselves.
> 
> Also--unlike other color.* configurations--color.man=always does not
> make any sense here; `git help` is always run for a tty (it would be very
> strange for a user to do `git help $page > output`, but in fact, that
> works anyway, we don't even need to check if stdout is a tty, but just
> to be consistent we do). So it's simply a boolean in our case.
> 
> So, in order for this change to have any effect:
> 
>   1. The user must use less
>   2. Not have the same LESS_TERMCAP variables set
>   3. Have color.ui enabled
>   4. Not have color.pager disabled
>   5. Not have color.man disabled
>   7. Not have git with stdout directed to a file
> 
> Fortunately the vast majority of our users meet all of the above, and
> anybody who doesn't would not be affected negatively (plus very likely
> comprises a very tiny minority).
> 
> [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
> 
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Phillip Wood <phillip.wood123@gmail.com>

This footer seems to have got broken between v5 and v6 - were you 
intending to delete it (which is fine by me) as my comments were about 
the approach of the last patch?

I'm still not convinced that git should be messing with the appearance 
of man pages but I don't think we're ever going to agree on that.

Best Wishes

Phillip

> Comments-by: Jeff King <peff@peff.net>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> 
> I returned back to the LESS_TERMCAP variables because -D options only
> exists for non-dos systems since 2021.
> 
> I tested this with a version of less that is 10 years old, and everything
> works as expected.
> 
> Also, I noticed less already chooses nice colors for standout with
> --use-color, and the prompt color is cyan, which is the fist one
> visible, so I changed standout to cyan.
> 
> Range-diff against v5:
> 1:  64c93d501f ! 1:  3a3e2837ad help: colorize man pages
>      @@ Commit message
>           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.
>      +    We can set LESS_TERMCAP variables to render bold and underlined text
>      +    with colors in the pager; a common trick[1].
>       
>           Bold is rendered as red, underlined as blue, and standout (prompt and
>      -    highlighted search) as inverse magenta.
>      +    highlighted search) as inverse cyan.
>       
>           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. The same if any LESS_TERMCAP_*
>      -    variables are set.
>      +    If the user already has LESS_TERMCAP variables set in his/her
>      +    environment, those are respected and nothing changes.
>       
>           A new color configuration is added: `color.man` for the people that want
>           to turn this feature off, otherwise `color.ui` is respected.
>      @@ Commit message
>           So, in order for this change to have any effect:
>       
>            1. The user must use less
>      -     2. Not have the LESS variable set
>      +     2. Not have the same LESS_TERMCAP variables set
>            3. Have color.ui enabled
>            4. Not have color.pager disabled
>            5. Not have color.man disabled
>      @@ Commit message
>           anybody who doesn't would not be affected negatively (plus very likely
>           comprises a very tiny minority).
>       
>      +    [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
>      +
>           Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>           Phillip Wood <phillip.wood123@gmail.com>
>           Comments-by: Jeff King <peff@peff.net>
>      @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag
>       +	if (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)
>       +		return;
>       +
>      -+	/* User already has configured less colors */
>      -+	if (getenv("LESS_TERMCAP_md") ||
>      -+		getenv("LESS_TERMCAP_us") ||
>      -+		getenv("LESS_TERMCAP_so")) {
>      -+		return;
>      -+	}
>      -+
>       +	/* Disable groff colors */
>       +	setenv("GROFF_NO_SGR", "1", 0);
>       +
>      -+	/* Add red to bold, blue to underline, and magenta to standout */
>      -+	/* No visual information is lost */
>      -+	setenv("LESS", "Dd+r$Du+b$Ds+m", 0);
>      ++	/* Bold */
>      ++	setenv("LESS_TERMCAP_md", GIT_COLOR_BOLD_RED, 0);
>      ++	setenv("LESS_TERMCAP_me", GIT_COLOR_RESET, 0);
>      ++
>      ++	/* Underline */
>      ++	setenv("LESS_TERMCAP_us", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);
>      ++	setenv("LESS_TERMCAP_ue", GIT_COLOR_RESET, 0);
>      ++
>      ++	/* Standout */
>      ++	setenv("LESS_TERMCAP_so", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);
>      ++	setenv("LESS_TERMCAP_se", GIT_COLOR_RESET, 0);
>       +}
>       +
>        static void exec_man_man(const char *path, const char *page)
>      @@ builtin/help.c: static int git_help_config(const char *var, const char *value, v
>        }
>        
>        static struct cmdnames main_cmds, other_cmds;
>      +
>      + ## color.h ##
>      +@@ color.h: struct strbuf;
>      + #define GIT_COLOR_FAINT		"\033[2m"
>      + #define GIT_COLOR_FAINT_ITALIC	"\033[2;3m"
>      + #define GIT_COLOR_REVERSE	"\033[7m"
>      ++#define GIT_COLOR_UNDERLINE	"\033[4m"
>      +
>      + /* A special value meaning "no color selected" */
>      + #define GIT_COLOR_NIL "NIL"
> 
>   Documentation/config/color.txt |  5 +++++
>   builtin/help.c                 | 32 +++++++++++++++++++++++++++++++-
>   color.h                        |  1 +
>   3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
> index d5daacb13a..11278b7f72 100644
> --- a/Documentation/config/color.txt
> +++ b/Documentation/config/color.txt
> @@ -126,6 +126,11 @@ color.interactive.<slot>::
>   	or `error`, for four distinct types of normal output from
>   	interactive commands.
>   
> +color.man::
> +	This flag can be used to disable the automatic colorizaton of man
> +	pages when using the less pager. It's activated only when color.ui
> +	allows it, and also when color.pager is on. (`true` by default).
> +
>   color.pager::
>   	A boolean to enable/disable colored output when the pager is in
>   	use (default is true).
> diff --git a/builtin/help.c b/builtin/help.c
> index bb339f0fc8..b6331afc2e 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -11,6 +11,7 @@
>   #include "config-list.h"
>   #include "help.h"
>   #include "alias.h"
> +#include "color.h"
>   
>   #ifndef DEFAULT_HELP_FORMAT
>   #define DEFAULT_HELP_FORMAT "man"
> @@ -43,6 +44,7 @@ static int verbose = 1;
>   static unsigned int colopts;
>   static enum help_format help_format = HELP_FORMAT_NONE;
>   static int exclude_guides;
> +static int man_color = 1;
>   static struct option builtin_help_options[] = {
>   	OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
>   	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
> @@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)
>   	}
>   }
>   
> +static void colorize_man(void)
> +{
> +	if (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)
> +		return;
> +
> +	/* Disable groff colors */
> +	setenv("GROFF_NO_SGR", "1", 0);
> +
> +	/* Bold */
> +	setenv("LESS_TERMCAP_md", GIT_COLOR_BOLD_RED, 0);
> +	setenv("LESS_TERMCAP_me", GIT_COLOR_RESET, 0);
> +
> +	/* Underline */
> +	setenv("LESS_TERMCAP_us", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);
> +	setenv("LESS_TERMCAP_ue", GIT_COLOR_RESET, 0);
> +
> +	/* Standout */
> +	setenv("LESS_TERMCAP_so", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);
> +	setenv("LESS_TERMCAP_se", GIT_COLOR_RESET, 0);
> +}
> +
>   static void exec_man_man(const char *path, const char *page)
>   {
>   	if (!path)
>   		path = "man";
> +
> +	colorize_man();
>   	execlp(path, "man", page, (char *)NULL);
>   	warning_errno(_("failed to exec '%s'"), path);
>   }
> @@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)
>   static void exec_man_cmd(const char *cmd, const char *page)
>   {
>   	struct strbuf shell_cmd = STRBUF_INIT;
> +	colorize_man();
>   	strbuf_addf(&shell_cmd, "%s %s", cmd, page);
>   	execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, (char *)NULL);
>   	warning(_("failed to exec '%s'"), cmd);
> @@ -371,8 +397,12 @@ static int git_help_config(const char *var, const char *value, void *cb)
>   	}
>   	if (starts_with(var, "man."))
>   		return add_man_viewer_info(var, value);
> +	if (!strcmp(var, "color.man")) {
> +		man_color = git_config_bool(var, value);
> +		return 0;
> +	}
>   
> -	return git_default_config(var, value, cb);
> +	return git_color_default_config(var, value, cb);
>   }
>   
>   static struct cmdnames main_cmds, other_cmds;
> diff --git a/color.h b/color.h
> index 98894d6a17..d012add4e8 100644
> --- a/color.h
> +++ b/color.h
> @@ -51,6 +51,7 @@ struct strbuf;
>   #define GIT_COLOR_FAINT		"\033[2m"
>   #define GIT_COLOR_FAINT_ITALIC	"\033[2;3m"
>   #define GIT_COLOR_REVERSE	"\033[7m"
> +#define GIT_COLOR_UNDERLINE	"\033[4m"
>   
>   /* A special value meaning "no color selected" */
>   #define GIT_COLOR_NIL "NIL"
>
Felipe Contreras May 24, 2021, 4:56 p.m. UTC | #2
Phillip Wood wrote:
> On 23/05/2021 06:44, Felipe Contreras wrote:

> > Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Phillip Wood <phillip.wood123@gmail.com>
> 
> This footer seems to have got broken between v5 and v6 - were you 
> intending to delete it (which is fine by me) as my comments were about 
> the approach of the last patch?

I was probably trying to put you in the Cc list.

> I'm still not convinced that git should be messing with the appearance 
> of man pages but I don't think we're ever going to agree on that.

That's your opinion, and it's fine, we all have opinions.

But the interesting thing for everyone else is *why*. Why aren't you
convinced?

I still haven't heard a convincing argument regarding what makes
`git help` fundamentally different from `git diff` _for the user_.

Cheers.
Ævar Arnfjörð Bjarmason June 8, 2021, 12:35 p.m. UTC | #3
On Sun, May 23 2021, Felipe Contreras wrote:

> We already colorize tools traditionally not colorized by default, like
> diff and grep. Let's do the same for man.
>
> 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 LESS_TERMCAP variables to render bold and underlined text
> with colors in the pager; a common trick[1].
>
> Bold is rendered as red, underlined as blue, and standout (prompt and
> highlighted search) as inverse cyan.
>
> Obviously this only works when the less pager is used.
>
> If the user already has LESS_TERMCAP variables set in his/her
> environment, those are respected and nothing changes.
>
> 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
> man. Therefore we need to check pager_use_color ourselves.
>
> Also--unlike other color.* configurations--color.man=always does not
> make any sense here; `git help` is always run for a tty (it would be very
> strange for a user to do `git help $page > output`, but in fact, that
> works anyway, we don't even need to check if stdout is a tty, but just
> to be consistent we do). So it's simply a boolean in our case.
>
> So, in order for this change to have any effect:
>
>  1. The user must use less
>  2. Not have the same LESS_TERMCAP variables set
>  3. Have color.ui enabled
>  4. Not have color.pager disabled
>  5. Not have color.man disabled
>  7. Not have git with stdout directed to a file
>
> Fortunately the vast majority of our users meet all of the above, and
> anybody who doesn't would not be affected negatively (plus very likely
> comprises a very tiny minority).
>
> [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
>
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Phillip Wood <phillip.wood123@gmail.com>
> Comments-by: Jeff King <peff@peff.net>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

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 for one would like to have this picked up by Junio.

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.

We have a wider audience running "next" than "seen" (but this didn't
even make "seen"), if this were to make it into a release and users
overwhelmingly dislike it it's no big deal. There's a config option to
turn it off, and/or we could make it opt-in later.

Alternatively this could be opt-in and not fall under the color.ui=auto
umbrella, or only in combination with feature.experimental (or a new
ui.experimental?, which would default to that?).

But in any case if judgement call UI changes are always hidden behind
options we'll never make forward progress on things that are possibly
better defaults (and if they're not, we can always simply revert the
change).
Junio C Hamano June 8, 2021, 1:57 p.m. UTC | #4
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> 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.

Thanks.
Felipe Contreras June 8, 2021, 5:48 p.m. UTC | #5
Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > 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",

Not true.

Jeff said users would probably have configured man to use colors
themselves, but he never responded back when I asked him *how* [1].

It is a tricky question, because I already know it's not possible to do
it in a way that works in all distributions, for all programs without
polluting the user environment to do things she probably doesn't want.

> 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).

Please explain exactly *how* the user will be able to do that.


Moreover. I don't think git should be in the business of educating users
how to use other software. The way they use less in conjunction with
other software is up to them.

And in fact we already help naive users that have not configured their
pager, so that it works better in git.

We do this for them:

  LESS=FRX LV=-c # see PAGER_ENV

Why aren't we "educating them" about LESS=FRX instead?

We have set good defaults for less since pretty much the start:

  f67b45f862 (Introduce trivial new pager.c helper infrastructure, 2006-02-28)


I don't think Jeff is the consensus. He expressed an opinion that
perhaps X is better, but without clearly defining X, that's not really
a viable option.

> I'd rather not to take it.

Can you explain why? All the outstanding comments have been addressed.

Cheers.

[1] https://lore.kernel.org/git/60a96e76a4b20_857e92085c@natae.notmuch/
Felipe Contreras June 24, 2021, 1:44 a.m. UTC | #6
> --- a/builtin/help.c
> +++ b/builtin/help.c

> @@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)
>  static void exec_man_cmd(const char *cmd, const char *page)
>  {
>  	struct strbuf shell_cmd = STRBUF_INIT;
> +	colorize_man();

On further reflection I don't think this colorize belongs here.
exec_man_cmd() is meant to execute any custom command, not necessarily
man.
diff mbox series

Patch

diff and grep. Let's do the same for man.

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 LESS_TERMCAP variables to render bold and underlined text
with colors in the pager; a common trick[1].

Bold is rendered as red, underlined as blue, and standout (prompt and
highlighted search) as inverse cyan.

Obviously this only works when the less pager is used.

If the user already has LESS_TERMCAP variables set in his/her
environment, those are respected and nothing changes.

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
man. Therefore we need to check pager_use_color ourselves.

Also--unlike other color.* configurations--color.man=always does not
make any sense here; `git help` is always run for a tty (it would be very
strange for a user to do `git help $page > output`, but in fact, that
works anyway, we don't even need to check if stdout is a tty, but just
to be consistent we do). So it's simply a boolean in our case.

So, in order for this change to have any effect:

 1. The user must use less
 2. Not have the same LESS_TERMCAP variables set
 3. Have color.ui enabled
 4. Not have color.pager disabled
 5. Not have color.man disabled
 7. Not have git with stdout directed to a file

Fortunately the vast majority of our users meet all of the above, and
anybody who doesn't would not be affected negatively (plus very likely
comprises a very tiny minority).

[1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Phillip Wood <phillip.wood123@gmail.com>
Comments-by: Jeff King <peff@peff.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

I returned back to the LESS_TERMCAP variables because -D options only
exists for non-dos systems since 2021.

I tested this with a version of less that is 10 years old, and everything
works as expected.

Also, I noticed less already chooses nice colors for standout with
--use-color, and the prompt color is cyan, which is the fist one
visible, so I changed standout to cyan.

Range-diff against v5:
1:  64c93d501f ! 1:  3a3e2837ad help: colorize man pages
    @@ Commit message
         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.
    +    We can set LESS_TERMCAP variables to render bold and underlined text
    +    with colors in the pager; a common trick[1].
     
         Bold is rendered as red, underlined as blue, and standout (prompt and
    -    highlighted search) as inverse magenta.
    +    highlighted search) as inverse cyan.
     
         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. The same if any LESS_TERMCAP_*
    -    variables are set.
    +    If the user already has LESS_TERMCAP variables set in his/her
    +    environment, those are respected and nothing changes.
     
         A new color configuration is added: `color.man` for the people that want
         to turn this feature off, otherwise `color.ui` is respected.
    @@ Commit message
         So, in order for this change to have any effect:
     
          1. The user must use less
    -     2. Not have the LESS variable set
    +     2. Not have the same LESS_TERMCAP variables set
          3. Have color.ui enabled
          4. Not have color.pager disabled
          5. Not have color.man disabled
    @@ Commit message
         anybody who doesn't would not be affected negatively (plus very likely
         comprises a very tiny minority).
     
    +    [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
    +
         Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Phillip Wood <phillip.wood123@gmail.com>
         Comments-by: Jeff King <peff@peff.net>
    @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag
     +	if (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)
     +		return;
     +
    -+	/* User already has configured less colors */
    -+	if (getenv("LESS_TERMCAP_md") ||
    -+		getenv("LESS_TERMCAP_us") ||
    -+		getenv("LESS_TERMCAP_so")) {
    -+		return;
    -+	}
    -+
     +	/* Disable groff colors */
     +	setenv("GROFF_NO_SGR", "1", 0);
     +
    -+	/* Add red to bold, blue to underline, and magenta to standout */
    -+	/* No visual information is lost */
    -+	setenv("LESS", "Dd+r$Du+b$Ds+m", 0);
    ++	/* Bold */
    ++	setenv("LESS_TERMCAP_md", GIT_COLOR_BOLD_RED, 0);
    ++	setenv("LESS_TERMCAP_me", GIT_COLOR_RESET, 0);
    ++
    ++	/* Underline */
    ++	setenv("LESS_TERMCAP_us", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);
    ++	setenv("LESS_TERMCAP_ue", GIT_COLOR_RESET, 0);
    ++
    ++	/* Standout */
    ++	setenv("LESS_TERMCAP_so", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);
    ++	setenv("LESS_TERMCAP_se", GIT_COLOR_RESET, 0);
     +}
     +
      static void exec_man_man(const char *path, const char *page)
    @@ builtin/help.c: static int git_help_config(const char *var, const char *value, v
      }
      
      static struct cmdnames main_cmds, other_cmds;
    +
    + ## color.h ##
    +@@ color.h: struct strbuf;
    + #define GIT_COLOR_FAINT		"\033[2m"
    + #define GIT_COLOR_FAINT_ITALIC	"\033[2;3m"
    + #define GIT_COLOR_REVERSE	"\033[7m"
    ++#define GIT_COLOR_UNDERLINE	"\033[4m"
    + 
    + /* A special value meaning "no color selected" */
    + #define GIT_COLOR_NIL "NIL"

 Documentation/config/color.txt |  5 +++++
 builtin/help.c                 | 32 +++++++++++++++++++++++++++++++-
 color.h                        |  1 +
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index d5daacb13a..11278b7f72 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -126,6 +126,11 @@  color.interactive.<slot>::
 	or `error`, for four distinct types of normal output from
 	interactive commands.
 
+color.man::
+	This flag can be used to disable the automatic colorizaton of man
+	pages when using the less pager. It's activated only when color.ui
+	allows it, and also when color.pager is on. (`true` by default).
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/builtin/help.c b/builtin/help.c
index bb339f0fc8..b6331afc2e 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -11,6 +11,7 @@ 
 #include "config-list.h"
 #include "help.h"
 #include "alias.h"
+#include "color.h"
 
 #ifndef DEFAULT_HELP_FORMAT
 #define DEFAULT_HELP_FORMAT "man"
@@ -43,6 +44,7 @@  static int verbose = 1;
 static unsigned int colopts;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
+static int man_color = 1;
 static struct option builtin_help_options[] = {
 	OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
@@ -253,10 +255,33 @@  static void exec_man_konqueror(const char *path, const char *page)
 	}
 }
 
+static void colorize_man(void)
+{
+	if (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)
+		return;
+
+	/* Disable groff colors */
+	setenv("GROFF_NO_SGR", "1", 0);
+
+	/* Bold */
+	setenv("LESS_TERMCAP_md", GIT_COLOR_BOLD_RED, 0);
+	setenv("LESS_TERMCAP_me", GIT_COLOR_RESET, 0);
+
+	/* Underline */
+	setenv("LESS_TERMCAP_us", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);
+	setenv("LESS_TERMCAP_ue", GIT_COLOR_RESET, 0);
+
+	/* Standout */
+	setenv("LESS_TERMCAP_so", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);
+	setenv("LESS_TERMCAP_se", GIT_COLOR_RESET, 0);
+}
+
 static void exec_man_man(const char *path, const char *page)
 {
 	if (!path)
 		path = "man";
+
+	colorize_man();
 	execlp(path, "man", page, (char *)NULL);
 	warning_errno(_("failed to exec '%s'"), path);
 }
@@ -264,6 +289,7 @@  static void exec_man_man(const char *path, const char *page)
 static void exec_man_cmd(const char *cmd, const char *page)
 {
 	struct strbuf shell_cmd = STRBUF_INIT;
+	colorize_man();
 	strbuf_addf(&shell_cmd, "%s %s", cmd, page);
 	execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, (char *)NULL);
 	warning(_("failed to exec '%s'"), cmd);
@@ -371,8 +397,12 @@  static int git_help_config(const char *var, const char *value, void *cb)
 	}
 	if (starts_with(var, "man."))
 		return add_man_viewer_info(var, value);
+	if (!strcmp(var, "color.man")) {
+		man_color = git_config_bool(var, value);
+		return 0;
+	}
 
-	return git_default_config(var, value, cb);
+	return git_color_default_config(var, value, cb);
 }
 
 static struct cmdnames main_cmds, other_cmds;
diff --git a/color.h b/color.h
index 98894d6a17..d012add4e8 100644
--- a/color.h
+++ b/color.h
@@ -51,6 +51,7 @@  struct strbuf;
 #define GIT_COLOR_FAINT		"\033[2m"
 #define GIT_COLOR_FAINT_ITALIC	"\033[2;3m"
 #define GIT_COLOR_REVERSE	"\033[7m"
+#define GIT_COLOR_UNDERLINE	"\033[4m"
 
 /* A special value meaning "no color selected" */
 #define GIT_COLOR_NIL "NIL"