From: Phillip Wood Date: Thu, 20 May 2021 09:26:03 GMT Subject: Re: [PATCH v4] help: colorize man pages Message-ID: <842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com> In-Reply-To: <20210520040725.133848-1-felipe.contreras@gmail.com> 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 - I would expect a man page to look the same whether it is displayed by 'man git foo' or 'git help foo' > 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. I've got LESS_TERMCAP_xx set and running LESS='Dd+r$Du+b$Ds' man git add changes the output colors > 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/ Best Wishes Phillip > 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 automatically [probably thanks to less being smart], 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. > > Moreover, just to be painstakingly comprehensive with people who have > color-aversion; we honour NO_COLOR [1]. > > So, in order for this change to have any effect: > > 1. The user must use less > 2. Not have the LESS variable set > 3. Have color.ui enabled > 4. Not have color.pager disabled > 5. Not have color.man disabled > 6. Not have NO_COLOR set > 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://no-color.org/ > > Suggested-by: Ævar Arnfjörð Bjarmason > Comments-by: Jeff King > Signed-off-by: Felipe Contreras > --- > Range-diff against v3: > 1: db93bf432b ! 1: 7249785014 help: colorize man pages > @@ Metadata > ## Commit message ## > help: colorize man pages > > + 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]. > + 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 (messages and > + Bold is rendered as red, underlined as blue, and standout (prompt and > highlighted search) as inverse magenta. > > - This only works when the pager is less. > + 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 not overwritten. > + If the user has already set the LESS variable in his/her environment, > + that is 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, and in > - addition color.pager needs to be turned on. > + 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; git help is always run for a tty (it would be very > + 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 automatically [probably thanks to less being smart], 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 to have an effect: > + Moreover, just to be painstakingly comprehensive with people who have > + color-aversion; we honour NO_COLOR [1]. > + > + 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 > + 2. Not have the LESS variable set > 3. Have color.ui enabled > - 4. Have color.pager enabled > + 4. Not have color.pager disabled > 5. Not have color.man disabled > - 6. Run git with stdout on a tty > + 6. Not have NO_COLOR set > + 7. Not have git with stdout directed to a file > > - Otherwise the current behavior remains. > + 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 > + [1] https://no-color.org/ > > Suggested-by: Ævar Arnfjörð Bjarmason > Comments-by: Jeff King > @@ 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; > + > ++ /* See: https://no-color.org/ */ > ++ if (getenv("NO_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_MAGENTA GIT_COLOR_REVERSE, 0); > -+ setenv("LESS_TERMCAP_se", GIT_COLOR_RESET, 0); > ++ /* Add red to bold, blue to underline, and magenta to standout */ > ++ /* No visual information is lost */ > ++ setenv("LESS", "Dd+r$Du+b$Ds", 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 | 28 +++++++++++++++++++++++++++- > 2 files changed, 32 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.:: > 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..298d97cc39 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,29 @@ 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; > + > + /* See: https://no-color.org/ */ > + if (getenv("NO_COLOR")) > + 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", 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 +285,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 +393,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; >