{"thread":{"id":"55761","subject":"[PATCH v6] help: colorize man pages","startedAt":"2021-05-23T05:44:58Z","lastAt":"2021-06-29T01:48:27Z","messageCount":22,"participants":["Felipe Contreras","Phillip Wood","Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":6,"patchTotal":null},"messages":[{"id":"425355","messageId":"20210523054454.1188757-1-felipe.contreras@gmail.com","threadId":"55761","inReplyTo":null,"subject":"[PATCH v6] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-23T05:44:54Z","receivedAt":"2021-05-23T05:44:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We already colorize tools traditionally not colorized by default, like\ndiff and grep. Let's do the same for man.\n\nOur man pages don't contain many useful colors (just blue links),\nmoreover, many people have groff SGR disabled, so they don't see any\ncolors with man pages.\n\nWe can set LESS_TERMCAP variables to render bold and underlined text\nwith colors in the pager; a common trick[1].\n\nBold is rendered as red, underlined as blue, and standout (prompt and\nhighlighted search) as inverse cyan.\n\nObviously this only works when the less pager is used.\n\nIf the user already has LESS_TERMCAP variables set in his/her\nenvironment, those are respected and nothing changes.\n\nA new color configuration is added: `color.man` for the people that want\nto turn this feature off, otherwise `color.ui` is respected.\nAdditionally, if color.pager is not enabled, this is disregarded.\n\nNormally check_auto_color() would check the value of `color.pager`, but\nin this particular case it's not git the one executing the pager, but\nman. Therefore we need to check pager_use_color ourselves.\n\nAlso--unlike other color.* configurations--color.man=always does not\nmake any sense here; `git help` is always run for a tty (it would be very\nstrange for a user to do `git help $page > output`, but in fact, that\nworks anyway, we don't even need to check if stdout is a tty, but just\nto be consistent we do). So it's simply a boolean in our case.\n\nSo, in order for this change to have any effect:\n\n 1. The user must use less\n 2. Not have the same LESS_TERMCAP variables set\n 3. Have color.ui enabled\n 4. Not have color.pager disabled\n 5. Not have color.man disabled\n 7. Not have git with stdout directed to a file\n\nFortunately the vast majority of our users meet all of the above, and\nanybody who doesn't would not be affected negatively (plus very likely\ncomprises a very tiny minority).\n\n[1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n\nSuggested-by: Ævar Arnfjörð Bjarmason \nPhillip Wood \nComments-by: Jeff King \nSigned-off-by: Felipe Contreras \n---\n\nI returned back to the LESS_TERMCAP variables because -D options only\nexists for non-dos systems since 2021.\n\nI tested this with a version of less that is 10 years old, and everything\nworks as expected.\n\nAlso, I noticed less already chooses nice colors for standout with\n--use-color, and the prompt color is cyan, which is the fist one\nvisible, so I changed standout to cyan.\n\nRange-diff against v5:\n1: 64c93d501f ! 1: 3a3e2837ad help: colorize man pages\n @@ Commit message\n moreover, many people have groff SGR disabled, so they don't see any\n colors with man pages.\n \n - We can set the LESS variable to render bold, underlined, and standout\n - text with colors in the less pager.\n + We can set LESS_TERMCAP variables to render bold and underlined text\n + with colors in the pager; a common trick[1].\n \n Bold is rendered as red, underlined as blue, and standout (prompt and\n - highlighted search) as inverse magenta.\n + highlighted search) as inverse cyan.\n \n Obviously this only works when the less pager is used.\n \n - If the user has already set the LESS variable in his/her environment,\n - that is respected, and nothing changes. The same if any LESS_TERMCAP_*\n - variables are set.\n + If the user already has LESS_TERMCAP variables set in his/her\n + environment, those are respected and nothing changes.\n \n A new color configuration is added: `color.man` for the people that want\n to turn this feature off, otherwise `color.ui` is respected.\n @@ Commit message\n So, in order for this change to have any effect:\n \n 1. The user must use less\n - 2. Not have the LESS variable set\n + 2. Not have the same LESS_TERMCAP variables set\n 3. Have color.ui enabled\n 4. Not have color.pager disabled\n 5. Not have color.man disabled\n @@ Commit message\n anybody who doesn't would not be affected negatively (plus very likely\n comprises a very tiny minority).\n \n + [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n +\n Suggested-by: Ævar Arnfjörð Bjarmason \n Phillip Wood \n Comments-by: Jeff King \n @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag\n +\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n +\t\treturn;\n +\n -+\t/* User already has configured less colors */\n -+\tif (getenv(\"LESS_TERMCAP_md\") ||\n -+\t\tgetenv(\"LESS_TERMCAP_us\") ||\n -+\t\tgetenv(\"LESS_TERMCAP_so\")) {\n -+\t\treturn;\n -+\t}\n -+\n +\t/* Disable groff colors */\n +\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n +\n -+\t/* Add red to bold, blue to underline, and magenta to standout */\n -+\t/* No visual information is lost */\n -+\tsetenv(\"LESS\", \"Dd+r$Du+b$Ds+m\", 0);\n ++\t/* Bold */\n ++\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n ++\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n ++\n ++\t/* Underline */\n ++\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n ++\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n ++\n ++\t/* Standout */\n ++\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n ++\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n +}\n +\n static void exec_man_man(const char *path, const char *page)\n @@ builtin/help.c: static int git_help_config(const char *var, const char *value, v\n }\n \n static struct cmdnames main_cmds, other_cmds;\n +\n + ## color.h ##\n +@@ color.h: struct strbuf;\n + #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n + #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n + #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n ++#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n + \n + /* A special value meaning \"no color selected\" */\n + #define GIT_COLOR_NIL \"NIL\"\n\n Documentation/config/color.txt | 5 +++++\n builtin/help.c | 32 +++++++++++++++++++++++++++++++-\n color.h | 1 +\n 3 files changed, 37 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\nindex d5daacb13a..11278b7f72 100644\n--- a/Documentation/config/color.txt\n+++ b/Documentation/config/color.txt\n@@ -126,6 +126,11 @@ color.interactive.::\n \tor `error`, for four distinct types of normal output from\n \tinteractive commands.\n \n+color.man::\n+\tThis flag can be used to disable the automatic colorizaton of man\n+\tpages when using the less pager. It's activated only when color.ui\n+\tallows it, and also when color.pager is on. (`true` by default).\n+\n color.pager::\n \tA boolean to enable/disable colored output when the pager is in\n \tuse (default is true).\ndiff --git a/builtin/help.c b/builtin/help.c\nindex bb339f0fc8..b6331afc2e 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -11,6 +11,7 @@\n #include \"config-list.h\"\n #include \"help.h\"\n #include \"alias.h\"\n+#include \"color.h\"\n \n #ifndef DEFAULT_HELP_FORMAT\n #define DEFAULT_HELP_FORMAT \"man\"\n@@ -43,6 +44,7 @@ static int verbose = 1;\n static unsigned int colopts;\n static enum help_format help_format = HELP_FORMAT_NONE;\n static int exclude_guides;\n+static int man_color = 1;\n static struct option builtin_help_options[] = {\n \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n \tOPT_HIDDEN_BOOL(0, \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n@@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)\n \t}\n }\n \n+static void colorize_man(void)\n+{\n+\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n+\t\treturn;\n+\n+\t/* Disable groff colors */\n+\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n+\n+\t/* Bold */\n+\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n+\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n+\n+\t/* Underline */\n+\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n+\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n+\n+\t/* Standout */\n+\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n+\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n+}\n+\n static void exec_man_man(const char *path, const char *page)\n {\n \tif (!path)\n \t\tpath = \"man\";\n+\n+\tcolorize_man();\n \texeclp(path, \"man\", page, (char *)NULL);\n \twarning_errno(_(\"failed to exec '%s'\"), path);\n }\n@@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)\n static void exec_man_cmd(const char *cmd, const char *page)\n {\n \tstruct strbuf shell_cmd = STRBUF_INIT;\n+\tcolorize_man();\n \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n \texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n \twarning(_(\"failed to exec '%s'\"), cmd);\n@@ -371,8 +397,12 @@ static int git_help_config(const char *var, const char *value, void *cb)\n \t}\n \tif (starts_with(var, \"man.\"))\n \t\treturn add_man_viewer_info(var, value);\n+\tif (!strcmp(var, \"color.man\")) {\n+\t\tman_color = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static struct cmdnames main_cmds, other_cmds;\ndiff --git a/color.h b/color.h\nindex 98894d6a17..d012add4e8 100644\n--- a/color.h\n+++ b/color.h\n@@ -51,6 +51,7 @@ struct strbuf;\n #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n+#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n \n /* A special value meaning \"no color selected\" */\n #define GIT_COLOR_NIL \"NIL\"\n-- \n2.32.0.rc0\n\n"},{"id":"425431","messageId":"7644ac29-2a65-e186-ff40-630dd500e160@gmail.com","threadId":"55761","inReplyTo":"20210523054454.1188757-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v6] help: colorize man pages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-05-24T13:13:53Z","receivedAt":"2021-05-24T13:13:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 23/05/2021 06:44, Felipe Contreras wrote:\n> We already colorize tools traditionally not colorized by default, like\n> diff and grep. Let's do the same for man.\n> \n> Our man pages don't contain many useful colors (just blue links),\n> moreover, many people have groff SGR disabled, so they don't see any\n> colors with man pages.\n> \n> We can set LESS_TERMCAP variables to render bold and underlined text\n> with colors in the pager; a common trick[1].\n> \n> Bold is rendered as red, underlined as blue, and standout (prompt and\n> highlighted search) as inverse cyan.\n> \n> Obviously this only works when the less pager is used.\n> \n> If the user already has LESS_TERMCAP variables set in his/her\n> environment, those are respected and nothing changes.\n> \n> A new color configuration is added: `color.man` for the people that want\n> to turn this feature off, otherwise `color.ui` is respected.\n> Additionally, if color.pager is not enabled, this is disregarded.\n> \n> Normally check_auto_color() would check the value of `color.pager`, but\n> in this particular case it's not git the one executing the pager, but\n> man. Therefore we need to check pager_use_color ourselves.\n> \n> Also--unlike other color.* configurations--color.man=always does not\n> make any sense here; `git help` is always run for a tty (it would be very\n> strange for a user to do `git help $page > output`, but in fact, that\n> works anyway, we don't even need to check if stdout is a tty, but just\n> to be consistent we do). So it's simply a boolean in our case.\n> \n> So, in order for this change to have any effect:\n> \n> 1. The user must use less\n> 2. Not have the same LESS_TERMCAP variables set\n> 3. Have color.ui enabled\n> 4. Not have color.pager disabled\n> 5. Not have color.man disabled\n> 7. Not have git with stdout directed to a file\n> \n> Fortunately the vast majority of our users meet all of the above, and\n> anybody who doesn't would not be affected negatively (plus very likely\n> comprises a very tiny minority).\n> \n> [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n> \n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Phillip Wood \n\nThis footer seems to have got broken between v5 and v6 - were you \nintending to delete it (which is fine by me) as my comments were about \nthe approach of the last patch?\n\nI'm still not convinced that git should be messing with the appearance \nof man pages but I don't think we're ever going to agree on that.\n\nBest Wishes\n\nPhillip\n\n> Comments-by: Jeff King \n> Signed-off-by: Felipe Contreras \n> ---\n> \n> I returned back to the LESS_TERMCAP variables because -D options only\n> exists for non-dos systems since 2021.\n> \n> I tested this with a version of less that is 10 years old, and everything\n> works as expected.\n> \n> Also, I noticed less already chooses nice colors for standout with\n> --use-color, and the prompt color is cyan, which is the fist one\n> visible, so I changed standout to cyan.\n> \n> Range-diff against v5:\n> 1: 64c93d501f ! 1: 3a3e2837ad help: colorize man pages\n> @@ Commit message\n> moreover, many people have groff SGR disabled, so they don't see any\n> colors with man pages.\n> \n> - We can set the LESS variable to render bold, underlined, and standout\n> - text with colors in the less pager.\n> + We can set LESS_TERMCAP variables to render bold and underlined text\n> + with colors in the pager; a common trick[1].\n> \n> Bold is rendered as red, underlined as blue, and standout (prompt and\n> - highlighted search) as inverse magenta.\n> + highlighted search) as inverse cyan.\n> \n> Obviously this only works when the less pager is used.\n> \n> - If the user has already set the LESS variable in his/her environment,\n> - that is respected, and nothing changes. The same if any LESS_TERMCAP_*\n> - variables are set.\n> + If the user already has LESS_TERMCAP variables set in his/her\n> + environment, those are respected and nothing changes.\n> \n> A new color configuration is added: `color.man` for the people that want\n> to turn this feature off, otherwise `color.ui` is respected.\n> @@ Commit message\n> So, in order for this change to have any effect:\n> \n> 1. The user must use less\n> - 2. Not have the LESS variable set\n> + 2. Not have the same LESS_TERMCAP variables set\n> 3. Have color.ui enabled\n> 4. Not have color.pager disabled\n> 5. Not have color.man disabled\n> @@ Commit message\n> anybody who doesn't would not be affected negatively (plus very likely\n> comprises a very tiny minority).\n> \n> + [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n> +\n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Phillip Wood \n> Comments-by: Jeff King \n> @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag\n> +\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n> +\t\treturn;\n> +\n> -+\t/* User already has configured less colors */\n> -+\tif (getenv(\"LESS_TERMCAP_md\") ||\n> -+\t\tgetenv(\"LESS_TERMCAP_us\") ||\n> -+\t\tgetenv(\"LESS_TERMCAP_so\")) {\n> -+\t\treturn;\n> -+\t}\n> -+\n> +\t/* Disable groff colors */\n> +\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n> +\n> -+\t/* Add red to bold, blue to underline, and magenta to standout */\n> -+\t/* No visual information is lost */\n> -+\tsetenv(\"LESS\", \"Dd+r$Du+b$Ds+m\", 0);\n> ++\t/* Bold */\n> ++\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n> ++\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n> ++\n> ++\t/* Underline */\n> ++\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n> ++\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n> ++\n> ++\t/* Standout */\n> ++\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n> ++\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n> +}\n> +\n> static void exec_man_man(const char *path, const char *page)\n> @@ builtin/help.c: static int git_help_config(const char *var, const char *value, v\n> }\n> \n> static struct cmdnames main_cmds, other_cmds;\n> +\n> + ## color.h ##\n> +@@ color.h: struct strbuf;\n> + #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n> + #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n> + #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n> ++#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n> +\n> + /* A special value meaning \"no color selected\" */\n> + #define GIT_COLOR_NIL \"NIL\"\n> \n> Documentation/config/color.txt | 5 +++++\n> builtin/help.c | 32 +++++++++++++++++++++++++++++++-\n> color.h | 1 +\n> 3 files changed, 37 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\n> index d5daacb13a..11278b7f72 100644\n> --- a/Documentation/config/color.txt\n> +++ b/Documentation/config/color.txt\n> @@ -126,6 +126,11 @@ color.interactive.::\n> \tor `error`, for four distinct types of normal output from\n> \tinteractive commands.\n> \n> +color.man::\n> +\tThis flag can be used to disable the automatic colorizaton of man\n> +\tpages when using the less pager. It's activated only when color.ui\n> +\tallows it, and also when color.pager is on. (`true` by default).\n> +\n> color.pager::\n> \tA boolean to enable/disable colored output when the pager is in\n> \tuse (default is true).\n> diff --git a/builtin/help.c b/builtin/help.c\n> index bb339f0fc8..b6331afc2e 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -11,6 +11,7 @@\n> #include \"config-list.h\"\n> #include \"help.h\"\n> #include \"alias.h\"\n> +#include \"color.h\"\n> \n> #ifndef DEFAULT_HELP_FORMAT\n> #define DEFAULT_HELP_FORMAT \"man\"\n> @@ -43,6 +44,7 @@ static int verbose = 1;\n> static unsigned int colopts;\n> static enum help_format help_format = HELP_FORMAT_NONE;\n> static int exclude_guides;\n> +static int man_color = 1;\n> static struct option builtin_help_options[] = {\n> \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> \tOPT_HIDDEN_BOOL(0, \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n> @@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)\n> \t}\n> }\n> \n> +static void colorize_man(void)\n> +{\n> +\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n> +\t\treturn;\n> +\n> +\t/* Disable groff colors */\n> +\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n> +\n> +\t/* Bold */\n> +\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n> +\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n> +\n> +\t/* Underline */\n> +\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n> +\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n> +\n> +\t/* Standout */\n> +\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n> +\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n> +}\n> +\n> static void exec_man_man(const char *path, const char *page)\n> {\n> \tif (!path)\n> \t\tpath = \"man\";\n> +\n> +\tcolorize_man();\n> \texeclp(path, \"man\", page, (char *)NULL);\n> \twarning_errno(_(\"failed to exec '%s'\"), path);\n> }\n> @@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)\n> static void exec_man_cmd(const char *cmd, const char *page)\n> {\n> \tstruct strbuf shell_cmd = STRBUF_INIT;\n> +\tcolorize_man();\n> \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n> \texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n> \twarning(_(\"failed to exec '%s'\"), cmd);\n> @@ -371,8 +397,12 @@ static int git_help_config(const char *var, const char *value, void *cb)\n> \t}\n> \tif (starts_with(var, \"man.\"))\n> \t\treturn add_man_viewer_info(var, value);\n> +\tif (!strcmp(var, \"color.man\")) {\n> +\t\tman_color = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> \n> -\treturn git_default_config(var, value, cb);\n> +\treturn git_color_default_config(var, value, cb);\n> }\n> \n> static struct cmdnames main_cmds, other_cmds;\n> diff --git a/color.h b/color.h\n> index 98894d6a17..d012add4e8 100644\n> --- a/color.h\n> +++ b/color.h\n> @@ -51,6 +51,7 @@ struct strbuf;\n> #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n> #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n> #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n> +#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n> \n> /* A special value meaning \"no color selected\" */\n> #define GIT_COLOR_NIL \"NIL\"\n> \n\n"},{"id":"425439","messageId":"60abdaa5bae55_1b2092082c@natae.notmuch","threadId":"55761","inReplyTo":"7644ac29-2a65-e186-ff40-630dd500e160@gmail.com","subject":"Re: [PATCH v6] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-24T16:56:05Z","receivedAt":"2021-05-24T16:56:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 23/05/2021 06:44, Felipe Contreras wrote:\n\n> > Suggested-by: Ævar Arnfjörð Bjarmason \n> > Phillip Wood \n> \n> This footer seems to have got broken between v5 and v6 - were you \n> intending to delete it (which is fine by me) as my comments were about \n> the approach of the last patch?\n\nI was probably trying to put you in the Cc list.\n\n> I'm still not convinced that git should be messing with the appearance \n> of man pages but I don't think we're ever going to agree on that.\n\nThat's your opinion, and it's fine, we all have opinions.\n\nBut the interesting thing for everyone else is *why*. Why aren't you\nconvinced?\n\nI still haven't heard a convincing argument regarding what makes\n`git help` fundamentally different from `git diff` _for the user_.\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"426768","messageId":"87o8cg34t8.fsf@evledraar.gmail.com","threadId":"55761","inReplyTo":"20210523054454.1188757-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v6] help: colorize man pages","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-08T12:35:24Z","receivedAt":"2021-06-08T12:43:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, May 23 2021, Felipe Contreras wrote:\n\n> We already colorize tools traditionally not colorized by default, like\n> diff and grep. Let's do the same for man.\n>\n> Our man pages don't contain many useful colors (just blue links),\n> moreover, many people have groff SGR disabled, so they don't see any\n> colors with man pages.\n>\n> We can set LESS_TERMCAP variables to render bold and underlined text\n> with colors in the pager; a common trick[1].\n>\n> Bold is rendered as red, underlined as blue, and standout (prompt and\n> highlighted search) as inverse cyan.\n>\n> Obviously this only works when the less pager is used.\n>\n> If the user already has LESS_TERMCAP variables set in his/her\n> environment, those are respected and nothing changes.\n>\n> A new color configuration is added: `color.man` for the people that want\n> to turn this feature off, otherwise `color.ui` is respected.\n> Additionally, if color.pager is not enabled, this is disregarded.\n>\n> Normally check_auto_color() would check the value of `color.pager`, but\n> in this particular case it's not git the one executing the pager, but\n> man. Therefore we need to check pager_use_color ourselves.\n>\n> Also--unlike other color.* configurations--color.man=always does not\n> make any sense here; `git help` is always run for a tty (it would be very\n> strange for a user to do `git help $page > output`, but in fact, that\n> works anyway, we don't even need to check if stdout is a tty, but just\n> to be consistent we do). So it's simply a boolean in our case.\n>\n> So, in order for this change to have any effect:\n>\n> 1. The user must use less\n> 2. Not have the same LESS_TERMCAP variables set\n> 3. Have color.ui enabled\n> 4. Not have color.pager disabled\n> 5. Not have color.man disabled\n> 7. Not have git with stdout directed to a file\n>\n> Fortunately the vast majority of our users meet all of the above, and\n> anybody who doesn't would not be affected negatively (plus very likely\n> comprises a very tiny minority).\n>\n> [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n>\n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Phillip Wood \n> Comments-by: Jeff King \n> Signed-off-by: Felipe Contreras \n> ---\n\nI've been running with this on my personal git build since May 26th. I\nhaven't had any issues with it, and I like the new coloring.\n\nI for one would like to have this picked up by Junio.\n\nI think this is a good example of a change that we're better off just\nmerging down and then reverting if the wider audience of git users hates\nit, rather than trying to come to some perfect consensus here\non-list.\n\nWe have a wider audience running \"next\" than \"seen\" (but this didn't\neven make \"seen\"), if this were to make it into a release and users\noverwhelmingly dislike it it's no big deal. There's a config option to\nturn it off, and/or we could make it opt-in later.\n\nAlternatively this could be opt-in and not fall under the color.ui=auto\numbrella, or only in combination with feature.experimental (or a new\nui.experimental?, which would default to that?).\n\nBut in any case if judgement call UI changes are always hidden behind\noptions we'll never make forward progress on things that are possibly\nbetter defaults (and if they're not, we can always simply revert the\nchange).\n"},{"id":"426776","messageId":"xmqq4ke8pig9.fsf@gitster.g","threadId":"55761","inReplyTo":"87o8cg34t8.fsf@evledraar.gmail.com","subject":"Re: [PATCH v6] help: colorize man pages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-08T13:57:42Z","receivedAt":"2021-06-08T13:57:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I've been running with this on my personal git build since May 26th. I\n> haven't had any issues with it, and I like the new coloring.\n> ...\n> I think this is a good example of a change that we're better off just\n> merging down and then reverting if the wider audience of git users hates\n> it, rather than trying to come to some perfect consensus here\n> on-list.\n\nMy impression was tht we already had a rough consensus here on-list\nthat it may be good to educate users who like this \"new coloring\"\nlike you do to configure their \"less\", so that they consistently get\nthe \"new coloring\" they like whether they are doing \"git help git\",\n\"man git\", or even \"man ls\", and the approach the posted patch takes\nwill not help (it only affects \"git help git\" among these).\n\nI'd rather not to take it.\n\nThanks.\n"},{"id":"426798","messageId":"60bfad5f170df_1abb47208c1@natae.notmuch","threadId":"55761","inReplyTo":"xmqq4ke8pig9.fsf@gitster.g","subject":"Re: [PATCH v6] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-08T17:48:15Z","receivedAt":"2021-06-08T17:49:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > I've been running with this on my personal git build since May 26th. I\n> > haven't had any issues with it, and I like the new coloring.\n> > ...\n> > I think this is a good example of a change that we're better off just\n> > merging down and then reverting if the wider audience of git users hates\n> > it, rather than trying to come to some perfect consensus here\n> > on-list.\n> \n> My impression was tht we already had a rough consensus here on-list\n> that it may be good to educate users who like this \"new coloring\"\n> like you do to configure their \"less\",\n\nNot true.\n\nJeff said users would probably have configured man to use colors\nthemselves, but he never responded back when I asked him *how* [1].\n\nIt is a tricky question, because I already know it's not possible to do\nit in a way that works in all distributions, for all programs without\npolluting the user environment to do things she probably doesn't want.\n\n> so that they consistently get the \"new coloring\" they like whether\n> they are doing \"git help git\", \"man git\", or even \"man ls\", and the\n> approach the posted patch takes will not help (it only affects \"git\n> help git\" among these).\n\nPlease explain exactly *how* the user will be able to do that.\n\n\nMoreover. I don't think git should be in the business of educating users\nhow to use other software. The way they use less in conjunction with\nother software is up to them.\n\nAnd in fact we already help naive users that have not configured their\npager, so that it works better in git.\n\nWe do this for them:\n\n LESS=FRX LV=-c # see PAGER_ENV\n\nWhy aren't we \"educating them\" about LESS=FRX instead?\n\nWe have set good defaults for less since pretty much the start:\n\n f67b45f862 (Introduce trivial new pager.c helper infrastructure, 2006-02-28)\n\n\nI don't think Jeff is the consensus. He expressed an opinion that\nperhaps X is better, but without clearly defining X, that's not really\na viable option.\n\n> I'd rather not to take it.\n\nCan you explain why? All the outstanding comments have been addressed.\n\nCheers.\n\n[1] https://lore.kernel.org/git/60a96e76a4b20_857e92085c@natae.notmuch/\n\n-- \nFelipe Contreras"},{"id":"428023","messageId":"patch-1.1-a950ef49e28-20210621T083254Z-avarab@gmail.com","threadId":"55761","inReplyTo":"xmqq4ke8pig9.fsf@gitster.g","subject":"[PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-21T08:34:00Z","receivedAt":"2021-06-21T08:34:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Felipe Contreras \n\nWe already colorize tools traditionally not colorized by default, like\ndiff and grep. Let's do the same for man, but only if `color.man` is\nexplicitly set to \"true\".\n\nUnlike other `color.*` output this colorization is not enabled by\n`color.ui` being true, the user needs to explicitly set the\n`color.man` variable to `true.\n\nWhen it was proposed to treat `color.man` like any other `color.*`\nvariable some thought that git opting in coloring for an external\nprogram such as man(1) was a step too far[1], even if the user invoked\nit via the \"git help \" wrapper.\n\nSo let's make this explicitly opt-in for now. As noted in the\ndocumentation we're leaving ourselves an out to turn this on by\ndefault in the future, or e.g. putting it under the\nfeature.experimental umbrella. We probably won't, but let's not\npromise users that `color.man` will forever be a special-case.\n\nAs for what this actually does the effect of having this enabled is\nthat a documentation blurb like (some parts elided with \"[...]\"):\n\n\tNAME\n\t----\n\tgit-config - Get and set [...]\n\n\tSYNOPSIS\n\t--------\n\t[...]\n\t'git config' [] [...]\n\t[...]\n\tThe `--type=` option instructs 'git config' to ensure [...]\n\nWill have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\nconfig\" and other '-quoted parts in BLUE UNDERLINE instead of\nUNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\ninstead of BOLD. The \"Standout\" setting is then used for the user's\nown search bar (invoked with \"/\") and prompt. See [2] for more\nexamples\n\nNormally check_auto_color() would check the value of `color.pager`, but\nin this particular case it's not git the one executing the pager, but\nman. Therefore we need to check pager_use_color ourselves.\n\nWe do not need to support `color.man` being set to `always`; The `git\nhelp` command is always run for a tty (it would be very strange for a\nuser to do `git help $page > output`, but in fact, that works anyway,\nwe don't even need to check if stdout is a tty, but just to be\nconsistent we do). So it's simply a boolean in our case.\n\nSo, in order for this change to have any effect:\n\n 1. color.man=true must be set in the config\n 2. The user must use less\n 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n 4. Have color.ui enabled\n 5. Not have color.pager disabled\n 6. Not have git with stdout directed to a file\n\n1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n\nSuggested-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Felipe Contreras \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Tue, Jun 08 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I've been running with this on my personal git build since May 26th. I\n>> haven't had any issues with it, and I like the new coloring.\n>> ...\n>> I think this is a good example of a change that we're better off just\n>> merging down and then reverting if the wider audience of git users hates\n>> it, rather than trying to come to some perfect consensus here\n>> on-list.\n>\n> My impression was tht we already had a rough consensus here on-list\n> that it may be good to educate users who like this \"new coloring\"\n> like you do to configure their \"less\", so that they consistently get\n> the \"new coloring\" they like whether they are doing \"git help git\",\n> \"man git\", or even \"man ls\", and the approach the posted patch takes\n> will not help (it only affects \"git help git\" among these).\n>\n> I'd rather not to take it.\n\nFair enough, here's a version I think you and others will find\nacceptable then. It allows users like me who like this to explicitly\nopt-in via color.man=true.\n\nI also took the liberty of making some changes to the commit message\nto reword it for this new behavior, and to show an example of the sort\nof colors that change with this patch.\n\nThe only change to the patch itself is that now color_man is set to 0\nby default, not 1. I also moved its declaration so that it's with the\none other config variable, insted of with the other command-line\noptions to \"git help\".\n\nRange-diff against v6:\n1: e021ca1da21 ! 1: a950ef49e28 help: colorize man pages\n @@ Metadata\n Author: Felipe Contreras \n \n ## Commit message ##\n - help: colorize man pages\n + help: colorize man pages if man.color=true under less(1)\n \n We already colorize tools traditionally not colorized by default, like\n - diff and grep. Let's do the same for man.\n -\n - Our man pages don't contain many useful colors (just blue links),\n - moreover, many people have groff SGR disabled, so they don't see any\n - colors with man pages.\n -\n - We can set LESS_TERMCAP variables to render bold and underlined text\n - with colors in the pager; a common trick[1].\n -\n - Bold is rendered as red, underlined as blue, and standout (prompt and\n - highlighted search) as inverse cyan.\n -\n - Obviously this only works when the less pager is used.\n -\n - If the user already has LESS_TERMCAP variables set in his/her\n - environment, those are respected and nothing changes.\n -\n - A new color configuration is added: `color.man` for the people that want\n - to turn this feature off, otherwise `color.ui` is respected.\n - Additionally, if color.pager is not enabled, this is disregarded.\n + diff and grep. Let's do the same for man, but only if `color.man` is\n + explicitly set to \"true\".\n +\n + Unlike other `color.*` output this colorization is not enabled by\n + `color.ui` being true, the user needs to explicitly set the\n + `color.man` variable to `true.\n +\n + When it was proposed to treat `color.man` like any other `color.*`\n + variable some thought that git opting in coloring for an external\n + program such as man(1) was a step too far[1], even if the user invoked\n + it via the \"git help \" wrapper.\n +\n + So let's make this explicitly opt-in for now. As noted in the\n + documentation we're leaving ourselves an out to turn this on by\n + default in the future, or e.g. putting it under the\n + feature.experimental umbrella. We probably won't, but let's not\n + promise users that `color.man` will forever be a special-case.\n +\n + As for what this actually does the effect of having this enabled is\n + that a documentation blurb like (some parts elided with \"[...]\"):\n +\n + NAME\n + ----\n + git-config - Get and set [...]\n +\n + SYNOPSIS\n + --------\n + [...]\n + 'git config' [] [...]\n + [...]\n + The `--type=` option instructs 'git config' to ensure [...]\n +\n + Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n + config\" and other '-quoted parts in BLUE UNDERLINE instead of\n + UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n + instead of BOLD. The \"Standout\" setting is then used for the user's\n + own search bar (invoked with \"/\") and prompt. See [2] for more\n + examples\n \n Normally check_auto_color() would check the value of `color.pager`, but\n in this particular case it's not git the one executing the pager, but\n man. Therefore we need to check pager_use_color ourselves.\n \n - Also--unlike other color.* configurations--color.man=always does not\n - make any sense here; `git help` is always run for a tty (it would be very\n - strange for a user to do `git help $page > output`, but in fact, that\n - works anyway, we don't even need to check if stdout is a tty, but just\n - to be consistent we do). So it's simply a boolean in our case.\n + We do not need to support `color.man` being set to `always`; The `git\n + help` command is always run for a tty (it would be very strange for a\n + user to do `git help $page > output`, but in fact, that works anyway,\n + we don't even need to check if stdout is a tty, but just to be\n + consistent we do). So it's simply a boolean in our case.\n \n So, in order for this change to have any effect:\n \n - 1. The user must use less\n - 2. Not have the same LESS_TERMCAP variables set\n - 3. Have color.ui enabled\n - 4. Not have color.pager disabled\n - 5. Not have color.man disabled\n - 7. Not have git with stdout directed to a file\n -\n - Fortunately the vast majority of our users meet all of the above, and\n - anybody who doesn't would not be affected negatively (plus very likely\n - comprises a very tiny minority).\n + 1. color.man=true must be set in the config\n + 2. The user must use less\n + 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n + 4. Have color.ui enabled\n + 5. Not have color.pager disabled\n + 6. Not have git with stdout directed to a file\n \n - [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n + 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n + 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n \n Suggested-by: Ævar Arnfjörð Bjarmason \n - Phillip Wood \n - Comments-by: Jeff King \n Signed-off-by: Felipe Contreras \n + Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## Documentation/config/color.txt ##\n @@ Documentation/config/color.txt: color.interactive.::\n @@ Documentation/config/color.txt: color.interactive.::\n \tinteractive commands.\n \n +color.man::\n -+\tThis flag can be used to disable the automatic colorizaton of man\n -+\tpages when using the less pager. It's activated only when color.ui\n -+\tallows it, and also when color.pager is on. (`true` by default).\n ++\tThis flag can be used to enable the automatic colorizaton of man\n ++\tpages when using the less pager, `false` by default. When set to\n ++\t`true` it's activated only when `color.ui` allows it, and if\n ++\t`color.pager` enable (which it is by default).\n +\n color.pager::\n - \tA boolean to enable/disable colored output when the pager is in\n - \tuse (default is true).\n + \tA boolean to specify whether `auto` color modes should colorize\n + \toutput going to the pager. Defaults to true; set this to false\n +@@ Documentation/config/color.txt: color.ui::\n + \toutput not intended for machine consumption to use color, to\n + \t`true` or `auto` (this is the default since Git 1.8.4) if you\n + \twant such output to use color when written to the terminal.\n +++\n ++When set to `true` certain other `color.*` variables may still not be\n ++turned on unless explicitly enabled. Currently this only applies to\n ++`color.man`, see above. Such opt-in variables may be moved under the\n ++default `color.ui` umbrella in the future.\n \n ## builtin/help.c ##\n @@\n @@ builtin/help.c\n \n #ifndef DEFAULT_HELP_FORMAT\n #define DEFAULT_HELP_FORMAT \"man\"\n -@@ builtin/help.c: static int verbose = 1;\n - static unsigned int colopts;\n - static enum help_format help_format = HELP_FORMAT_NONE;\n - static int exclude_guides;\n -+static int man_color = 1;\n - static struct option builtin_help_options[] = {\n - \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n - \tOPT_HIDDEN_BOOL(0, \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n +@@ builtin/help.c: enum help_format {\n + \tHELP_FORMAT_WEB\n + };\n + \n ++static int man_color;\n + static const char *html_path;\n + \n + static int show_all = 0;\n @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *page)\n \t}\n }\n\n Documentation/config/color.txt | 11 +++++++++++\n builtin/help.c | 32 +++++++++++++++++++++++++++++++-\n color.h | 1 +\n 3 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\nindex e05d520a867..2f12ae3386d 100644\n--- a/Documentation/config/color.txt\n+++ b/Documentation/config/color.txt\n@@ -126,6 +126,12 @@ color.interactive.::\n \tor `error`, for four distinct types of normal output from\n \tinteractive commands.\n \n+color.man::\n+\tThis flag can be used to enable the automatic colorizaton of man\n+\tpages when using the less pager, `false` by default. When set to\n+\t`true` it's activated only when `color.ui` allows it, and if\n+\t`color.pager` enable (which it is by default).\n+\n color.pager::\n \tA boolean to specify whether `auto` color modes should colorize\n \toutput going to the pager. Defaults to true; set this to false\n@@ -200,3 +206,8 @@ color.ui::\n \toutput not intended for machine consumption to use color, to\n \t`true` or `auto` (this is the default since Git 1.8.4) if you\n \twant such output to use color when written to the terminal.\n++\n+When set to `true` certain other `color.*` variables may still not be\n+turned on unless explicitly enabled. Currently this only applies to\n+`color.man`, see above. Such opt-in variables may be moved under the\n+default `color.ui` umbrella in the future.\ndiff --git a/builtin/help.c b/builtin/help.c\nindex bb339f0fc80..c607da88d78 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -11,6 +11,7 @@\n #include \"config-list.h\"\n #include \"help.h\"\n #include \"alias.h\"\n+#include \"color.h\"\n \n #ifndef DEFAULT_HELP_FORMAT\n #define DEFAULT_HELP_FORMAT \"man\"\n@@ -34,6 +35,7 @@ enum help_format {\n \tHELP_FORMAT_WEB\n };\n \n+static int man_color;\n static const char *html_path;\n \n static int show_all = 0;\n@@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)\n \t}\n }\n \n+static void colorize_man(void)\n+{\n+\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n+\t\treturn;\n+\n+\t/* Disable groff colors */\n+\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n+\n+\t/* Bold */\n+\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n+\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n+\n+\t/* Underline */\n+\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n+\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n+\n+\t/* Standout */\n+\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n+\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n+}\n+\n static void exec_man_man(const char *path, const char *page)\n {\n \tif (!path)\n \t\tpath = \"man\";\n+\n+\tcolorize_man();\n \texeclp(path, \"man\", page, (char *)NULL);\n \twarning_errno(_(\"failed to exec '%s'\"), path);\n }\n@@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)\n static void exec_man_cmd(const char *cmd, const char *page)\n {\n \tstruct strbuf shell_cmd = STRBUF_INIT;\n+\tcolorize_man();\n \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n \texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n \twarning(_(\"failed to exec '%s'\"), cmd);\n@@ -371,8 +397,12 @@ static int git_help_config(const char *var, const char *value, void *cb)\n \t}\n \tif (starts_with(var, \"man.\"))\n \t\treturn add_man_viewer_info(var, value);\n+\tif (!strcmp(var, \"color.man\")) {\n+\t\tman_color = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static struct cmdnames main_cmds, other_cmds;\ndiff --git a/color.h b/color.h\nindex 98894d6a175..d012add4e8a 100644\n--- a/color.h\n+++ b/color.h\n@@ -51,6 +51,7 @@ struct strbuf;\n #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n+#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n \n /* A special value meaning \"no color selected\" */\n #define GIT_COLOR_NIL \"NIL\"\n-- \n2.32.0.599.g672d808136f\n\n"},{"id":"428028","messageId":"351215c0-98a9-dc39-1e19-d3d6dbb4bd56@gmail.com","threadId":"55761","inReplyTo":"patch-1.1-a950ef49e28-20210621T083254Z-avarab@gmail.com","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-06-21T10:17:13Z","receivedAt":"2021-06-21T10:17:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 21/06/2021 09:34, Ævar Arnfjörð Bjarmason wrote:\n> From: Felipe Contreras \n> \n> We already colorize tools traditionally not colorized by default, like\n> diff and grep. Let's do the same for man, but only if `color.man` is\n> explicitly set to \"true\".\n> \n> Unlike other `color.*` output this colorization is not enabled by\n> `color.ui` being true, the user needs to explicitly set the\n> `color.man` variable to `true.\n> \n> When it was proposed to treat `color.man` like any other `color.*`\n> variable some thought that git opting in coloring for an external\n> program such as man(1) was a step too far[1], even if the user invoked\n> it via the \"git help \" wrapper.\n> \n> So let's make this explicitly opt-in for now. As noted in the\n> documentation we're leaving ourselves an out to turn this on by\n> default in the future, or e.g. putting it under the\n> feature.experimental umbrella. We probably won't, but let's not\n> promise users that `color.man` will forever be a special-case.\n> \n> As for what this actually does the effect of having this enabled is\n> that a documentation blurb like (some parts elided with \"[...]\"):\n> \n> \tNAME\n> \t----\n> \tgit-config - Get and set [...]\n> \n> \tSYNOPSIS\n> \t--------\n> \t[...]\n> \t'git config' [] [...]\n> \t[...]\n> \tThe `--type=` option instructs 'git config' to ensure [...]\n> \n> Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n> config\" and other '-quoted parts in BLUE UNDERLINE instead of\n> UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n> instead of BOLD. The \"Standout\" setting is then used for the user's\n> own search bar (invoked with \"/\") and prompt. See [2] for more\n> examples\n> \n> Normally check_auto_color() would check the value of `color.pager`, but\n> in this particular case it's not git the one executing the pager, but\n> man. Therefore we need to check pager_use_color ourselves.\n> \n> We do not need to support `color.man` being set to `always`; The `git\n> help` command is always run for a tty (it would be very strange for a\n> user to do `git help $page > output`, but in fact, that works anyway,\n> we don't even need to check if stdout is a tty, but just to be\n> consistent we do). So it's simply a boolean in our case.\n> \n> So, in order for this change to have any effect:\n> \n> 1. color.man=true must be set in the config\n> 2. The user must use less\n> 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n> 4. Have color.ui enabled\n> 5. Not have color.pager disabled\n> 6. Not have git with stdout directed to a file\n> \n> 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n> 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n\nThanks for taking the time to write such a clear and comprehensive \nmessage, I think making this setting opt in for now makes sense. I've \nleft a couple of comments below.\n\n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n> On Tue, Jun 08 2021, Junio C Hamano wrote:\n> \n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> I've been running with this on my personal git build since May 26th. I\n>>> haven't had any issues with it, and I like the new coloring.\n>>> ...\n>>> I think this is a good example of a change that we're better off just\n>>> merging down and then reverting if the wider audience of git users hates\n>>> it, rather than trying to come to some perfect consensus here\n>>> on-list.\n>>\n>> My impression was tht we already had a rough consensus here on-list\n>> that it may be good to educate users who like this \"new coloring\"\n>> like you do to configure their \"less\", so that they consistently get\n>> the \"new coloring\" they like whether they are doing \"git help git\",\n>> \"man git\", or even \"man ls\", and the approach the posted patch takes\n>> will not help (it only affects \"git help git\" among these).\n>>\n>> I'd rather not to take it.\n> \n> Fair enough, here's a version I think you and others will find\n> acceptable then. It allows users like me who like this to explicitly\n> opt-in via color.man=true.\n> \n> I also took the liberty of making some changes to the commit message\n> to reword it for this new behavior, and to show an example of the sort\n> of colors that change with this patch.\n> \n> The only change to the patch itself is that now color_man is set to 0\n> by default, not 1. I also moved its declaration so that it's with the\n> one other config variable, insted of with the other command-line\n> options to \"git help\".\n> \n> Range-diff against v6:\n> 1: e021ca1da21 ! 1: a950ef49e28 help: colorize man pages\n> @@ Metadata\n> Author: Felipe Contreras \n> \n> ## Commit message ##\n> - help: colorize man pages\n> + help: colorize man pages if man.color=true under less(1)\n> \n> We already colorize tools traditionally not colorized by default, like\n> - diff and grep. Let's do the same for man.\n> -\n> - Our man pages don't contain many useful colors (just blue links),\n> - moreover, many people have groff SGR disabled, so they don't see any\n> - colors with man pages.\n> -\n> - We can set LESS_TERMCAP variables to render bold and underlined text\n> - with colors in the pager; a common trick[1].\n> -\n> - Bold is rendered as red, underlined as blue, and standout (prompt and\n> - highlighted search) as inverse cyan.\n> -\n> - Obviously this only works when the less pager is used.\n> -\n> - If the user already has LESS_TERMCAP variables set in his/her\n> - environment, those are respected and nothing changes.\n> -\n> - A new color configuration is added: `color.man` for the people that want\n> - to turn this feature off, otherwise `color.ui` is respected.\n> - Additionally, if color.pager is not enabled, this is disregarded.\n> + diff and grep. Let's do the same for man, but only if `color.man` is\n> + explicitly set to \"true\".\n> +\n> + Unlike other `color.*` output this colorization is not enabled by\n> + `color.ui` being true, the user needs to explicitly set the\n> + `color.man` variable to `true.\n> +\n> + When it was proposed to treat `color.man` like any other `color.*`\n> + variable some thought that git opting in coloring for an external\n> + program such as man(1) was a step too far[1], even if the user invoked\n> + it via the \"git help \" wrapper.\n> +\n> + So let's make this explicitly opt-in for now. As noted in the\n> + documentation we're leaving ourselves an out to turn this on by\n> + default in the future, or e.g. putting it under the\n> + feature.experimental umbrella. We probably won't, but let's not\n> + promise users that `color.man` will forever be a special-case.\n> +\n> + As for what this actually does the effect of having this enabled is\n> + that a documentation blurb like (some parts elided with \"[...]\"):\n> +\n> + NAME\n> + ----\n> + git-config - Get and set [...]\n> +\n> + SYNOPSIS\n> + --------\n> + [...]\n> + 'git config' [] [...]\n> + [...]\n> + The `--type=` option instructs 'git config' to ensure [...]\n> +\n> + Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n> + config\" and other '-quoted parts in BLUE UNDERLINE instead of\n> + UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n> + instead of BOLD. The \"Standout\" setting is then used for the user's\n> + own search bar (invoked with \"/\") and prompt. See [2] for more\n> + examples\n> \n> Normally check_auto_color() would check the value of `color.pager`, but\n> in this particular case it's not git the one executing the pager, but\n> man. Therefore we need to check pager_use_color ourselves.\n> \n> - Also--unlike other color.* configurations--color.man=always does not\n> - make any sense here; `git help` is always run for a tty (it would be very\n> - strange for a user to do `git help $page > output`, but in fact, that\n> - works anyway, we don't even need to check if stdout is a tty, but just\n> - to be consistent we do). So it's simply a boolean in our case.\n> + We do not need to support `color.man` being set to `always`; The `git\n> + help` command is always run for a tty (it would be very strange for a\n> + user to do `git help $page > output`, but in fact, that works anyway,\n> + we don't even need to check if stdout is a tty, but just to be\n> + consistent we do). So it's simply a boolean in our case.\n> \n> So, in order for this change to have any effect:\n> \n> - 1. The user must use less\n> - 2. Not have the same LESS_TERMCAP variables set\n> - 3. Have color.ui enabled\n> - 4. Not have color.pager disabled\n> - 5. Not have color.man disabled\n> - 7. Not have git with stdout directed to a file\n> -\n> - Fortunately the vast majority of our users meet all of the above, and\n> - anybody who doesn't would not be affected negatively (plus very likely\n> - comprises a very tiny minority).\n> + 1. color.man=true must be set in the config\n> + 2. The user must use less\n> + 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n> + 4. Have color.ui enabled\n> + 5. Not have color.pager disabled\n> + 6. Not have git with stdout directed to a file\n> \n> - [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n> + 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n> + 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n> \n> Suggested-by: Ævar Arnfjörð Bjarmason \n> - Phillip Wood \n> - Comments-by: Jeff King \n> Signed-off-by: Felipe Contreras \n> + Signed-off-by: Ævar Arnfjörð Bjarmason \n> \n> ## Documentation/config/color.txt ##\n> @@ Documentation/config/color.txt: color.interactive.::\n> @@ Documentation/config/color.txt: color.interactive.::\n> \tinteractive commands.\n> \n> +color.man::\n> -+\tThis flag can be used to disable the automatic colorizaton of man\n> -+\tpages when using the less pager. It's activated only when color.ui\n> -+\tallows it, and also when color.pager is on. (`true` by default).\n> ++\tThis flag can be used to enable the automatic colorizaton of man\n> ++\tpages when using the less pager, `false` by default. When set to\n> ++\t`true` it's activated only when `color.ui` allows it, and if\n> ++\t`color.pager` enable (which it is by default).\n> +\n> color.pager::\n> - \tA boolean to enable/disable colored output when the pager is in\n> - \tuse (default is true).\n> + \tA boolean to specify whether `auto` color modes should colorize\n> + \toutput going to the pager. Defaults to true; set this to false\n> +@@ Documentation/config/color.txt: color.ui::\n> + \toutput not intended for machine consumption to use color, to\n> + \t`true` or `auto` (this is the default since Git 1.8.4) if you\n> + \twant such output to use color when written to the terminal.\n> +++\n> ++When set to `true` certain other `color.*` variables may still not be\n> ++turned on unless explicitly enabled. Currently this only applies to\n> ++`color.man`, see above. Such opt-in variables may be moved under the\n> ++default `color.ui` umbrella in the future.\n> \n> ## builtin/help.c ##\n> @@\n> @@ builtin/help.c\n> \n> #ifndef DEFAULT_HELP_FORMAT\n> #define DEFAULT_HELP_FORMAT \"man\"\n> -@@ builtin/help.c: static int verbose = 1;\n> - static unsigned int colopts;\n> - static enum help_format help_format = HELP_FORMAT_NONE;\n> - static int exclude_guides;\n> -+static int man_color = 1;\n> - static struct option builtin_help_options[] = {\n> - \tOPT_BOOL('a', \"all\", &show_all, N_(\"print all available commands\")),\n> - \tOPT_HIDDEN_BOOL(0, \"exclude-guides\", &exclude_guides, N_(\"exclude guides\")),\n> +@@ builtin/help.c: enum help_format {\n> + \tHELP_FORMAT_WEB\n> + };\n> +\n> ++static int man_color;\n> + static const char *html_path;\n> +\n> + static int show_all = 0;\n> @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *page)\n> \t}\n> }\n> \n> Documentation/config/color.txt | 11 +++++++++++\n> builtin/help.c | 32 +++++++++++++++++++++++++++++++-\n> color.h | 1 +\n> 3 files changed, 43 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\n> index e05d520a867..2f12ae3386d 100644\n> --- a/Documentation/config/color.txt\n> +++ b/Documentation/config/color.txt\n> @@ -126,6 +126,12 @@ color.interactive.::\n> \tor `error`, for four distinct types of normal output from\n> \tinteractive commands.\n> \n> +color.man::\n> +\tThis flag can be used to enable the automatic colorizaton of man\n> +\tpages when using the less pager, `false` by default. When set to\n> +\t`true` it's activated only when `color.ui` allows it, and if\n> +\t`color.pager` enable (which it is by default).\n\ns/and if `color.pager` enable/and `color.pager` is enabled/?\n\n> +\n> color.pager::\n> \tA boolean to specify whether `auto` color modes should colorize\n> \toutput going to the pager. Defaults to true; set this to false\n> @@ -200,3 +206,8 @@ color.ui::\n> \toutput not intended for machine consumption to use color, to\n> \t`true` or `auto` (this is the default since Git 1.8.4) if you\n> \twant such output to use color when written to the terminal.\n> ++\n> +When set to `true` certain other `color.*` variables may still not be\n> +turned on unless explicitly enabled. Currently this only applies to\n> +`color.man`, see above. Such opt-in variables may be moved under the\n> +default `color.ui` umbrella in the future.\n> diff --git a/builtin/help.c b/builtin/help.c\n> index bb339f0fc80..c607da88d78 100644\n> --- a/builtin/help.c\n> +++ b/builtin/help.c\n> @@ -11,6 +11,7 @@\n> #include \"config-list.h\"\n> #include \"help.h\"\n> #include \"alias.h\"\n> +#include \"color.h\"\n> \n> #ifndef DEFAULT_HELP_FORMAT\n> #define DEFAULT_HELP_FORMAT \"man\"\n> @@ -34,6 +35,7 @@ enum help_format {\n> \tHELP_FORMAT_WEB\n> };\n> \n> +static int man_color;\n> static const char *html_path;\n> \n> static int show_all = 0;\n> @@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)\n> \t}\n> }\n> \n> +static void colorize_man(void)\n> +{\n> +\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n> +\t\treturn;\n> +\n> +\t/* Disable groff colors */\n> +\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n> +\n> +\t/* Bold */\n> +\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n> +\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n\nWe take care not to override the user's preference if these variables \nare set individually, but there is a possible corner case where the user \nsets a subset of these LESS_TERMCAP_* because they like the defaults for \nsome of them but not others, in that case we wouldn't want to set any of \nthem. Whether this is likely to be a problem in practice or not I'm not \nsure. Version 5 [1] of this patch took care not to override the users \nsettings in this case.\n\nBest Wishes\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/20210522011718.541986-1-felipe.contreras@gmail.com/\n\n> +\n> +\t/* Underline */\n> +\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n> +\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n> +\n> +\t/* Standout */\n> +\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n> +\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n> +}\n> +\n> static void exec_man_man(const char *path, const char *page)\n> {\n> \tif (!path)\n> \t\tpath = \"man\";\n> +\n> +\tcolorize_man();\n> \texeclp(path, \"man\", page, (char *)NULL);\n> \twarning_errno(_(\"failed to exec '%s'\"), path);\n> }\n> @@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)\n> static void exec_man_cmd(const char *cmd, const char *page)\n> {\n> \tstruct strbuf shell_cmd = STRBUF_INIT;\n> +\tcolorize_man();\n> \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n> \texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n> \twarning(_(\"failed to exec '%s'\"), cmd);\n> @@ -371,8 +397,12 @@ static int git_help_config(const char *var, const char *value, void *cb)\n> \t}\n> \tif (starts_with(var, \"man.\"))\n> \t\treturn add_man_viewer_info(var, value);\n> +\tif (!strcmp(var, \"color.man\")) {\n> +\t\tman_color = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n> \n> -\treturn git_default_config(var, value, cb);\n> +\treturn git_color_default_config(var, value, cb);\n> }\n> \n> static struct cmdnames main_cmds, other_cmds;\n> diff --git a/color.h b/color.h\n> index 98894d6a175..d012add4e8a 100644\n> --- a/color.h\n> +++ b/color.h\n> @@ -51,6 +51,7 @@ struct strbuf;\n> #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n> #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n> #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n> +#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n> \n> /* A special value meaning \"no color selected\" */\n> #define GIT_COLOR_NIL \"NIL\"\n> \n\n"},{"id":"428029","messageId":"xmqqfsxbika3.fsf@gitster.g","threadId":"55761","inReplyTo":"patch-1.1-a950ef49e28-20210621T083254Z-avarab@gmail.com","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-21T10:28:36Z","receivedAt":"2021-06-21T10:28:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> So, in order for this change to have any effect:\n>\n> 1. color.man=true must be set in the config\n> 2. The user must use less\n> 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n> 4. Have color.ui enabled\n> 5. Not have color.pager disabled\n> 6. Not have git with stdout directed to a file\n>\n> 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n> 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n>\n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n>\n> On Tue, Jun 08 2021, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> I've been running with this on my personal git build since May 26th. I\n>>> haven't had any issues with it, and I like the new coloring.\n>>> ...\n>>> I think this is a good example of a change that we're better off just\n>>> merging down and then reverting if the wider audience of git users hates\n>>> it, rather than trying to come to some perfect consensus here\n>>> on-list.\n>>\n>> My impression was tht we already had a rough consensus here on-list\n>> that it may be good to educate users who like this \"new coloring\"\n>> like you do to configure their \"less\", so that they consistently get\n>> the \"new coloring\" they like whether they are doing \"git help git\",\n>> \"man git\", or even \"man ls\", and the approach the posted patch takes\n>> will not help (it only affects \"git help git\" among these).\n>>\n>> I'd rather not to take it.\n>\n> Fair enough, here's a version I think you and others will find\n> acceptable then. It allows users like me who like this to explicitly\n> opt-in via color.man=true.\n\nNot really.\n\nSince the implementation of the posted patch, as I understand it,\ndoes not aim to affect both \"git help -m foo\" and \"man git-foo\"\nidentically, I think it would be easier to understand to end-users\nif this were exposed as a new \"mode\", like \"git help --web\" and \"git\nhelp --info\" are different modes from the \"git help --man\",\nsomething like \"git help --fancy-man\" (or whatever is easy to type\nand explain, and also add it to the variants help.format knows about\nto make it easy to set the default).\n\nOne advantage of doing so is that we do not have to worry about \"ah,\nuser has LESS_BLAH environment variable so we should disable this\nnew mode here\" etc. As long as the new mode is requested either via\nthe command line option or help.format configuration, it can\ncompletely take it over. That simplifies the necessary explanation\ngiven to the users quite a lot, no?\n\n> \t----\n> \tgit-config - Get and set [...]\n>\n> \tSYNOPSIS\n> \t--------\n> \t[...]\n> \t'git config' [] [...]\n> \t[...]\n> \tThe `--type=` option instructs 'git config' to ensure [...]\n>\n> Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n> config\" and other '-quoted parts in BLUE UNDERLINE instead of\n> UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n> instead of BOLD. The \"Standout\" setting is then used for the user's\n> own search bar (invoked with \"/\") and prompt. See [2] for more\n> examples\n\nThere are BOLD RED and READ BOLD; are they differently rendered?\n"},{"id":"428050","messageId":"60d0b75c80635_105b452089b@natae.notmuch","threadId":"55761","inReplyTo":"patch-1.1-a950ef49e28-20210621T083254Z-avarab@gmail.com","subject":"RE: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-21T15:59:24Z","receivedAt":"2021-06-21T15:59:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> From: Felipe Contreras \n> \n> We already colorize tools traditionally not colorized by default, like\n> diff and grep. Let's do the same for man, but only if `color.man` is\n> explicitly set to \"true\".\n> \n> Unlike other `color.*` output this colorization is not enabled by\n> `color.ui` being true, the user needs to explicitly set the\n> `color.man` variable to `true.\n> \n> When it was proposed to treat `color.man` like any other `color.*`\n> variable some thought that git opting in coloring for an external\n> program such as man(1) was a step too far[1], even if the user invoked\n> it via the \"git help \" wrapper.\n> \n> So let's make this explicitly opt-in for now. As noted in the\n> documentation we're leaving ourselves an out to turn this on by\n> default in the future, or e.g. putting it under the\n> feature.experimental umbrella. We probably won't, but let's not\n> promise users that `color.man` will forever be a special-case.\n> \n> As for what this actually does the effect of having this enabled is\n> that a documentation blurb like (some parts elided with \"[...]\"):\n> \n> \tNAME\n> \t----\n> \tgit-config - Get and set [...]\n> \n> \tSYNOPSIS\n> \t--------\n> \t[...]\n> \t'git config' [] [...]\n> \t[...]\n> \tThe `--type=` option instructs 'git config' to ensure [...]\n> \n> Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n> config\" and other '-quoted parts in BLUE UNDERLINE instead of\n> UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n> instead of BOLD. The \"Standout\" setting is then used for the user's\n> own search bar (invoked with \"/\") and prompt. See [2] for more\n> examples\n> \n> Normally check_auto_color() would check the value of `color.pager`, but\n> in this particular case it's not git the one executing the pager, but\n> man. Therefore we need to check pager_use_color ourselves.\n> \n> We do not need to support `color.man` being set to `always`; The `git\n> help` command is always run for a tty (it would be very strange for a\n> user to do `git help $page > output`, but in fact, that works anyway,\n> we don't even need to check if stdout is a tty, but just to be\n> consistent we do). So it's simply a boolean in our case.\n> \n> So, in order for this change to have any effect:\n> \n> 1. color.man=true must be set in the config\n> 2. The user must use less\n> 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n> 4. Have color.ui enabled\n> 5. Not have color.pager disabled\n> 6. Not have git with stdout directed to a file\n> \n> 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n> 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n> \n> Suggested-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n\nLooks good to me.\n\n-- \nFelipe Contreras"},{"id":"428087","messageId":"60d0dd619a2c6_10782120814@natae.notmuch","threadId":"55761","inReplyTo":"xmqqfsxbika3.fsf@gitster.g","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-21T18:41:37Z","receivedAt":"2021-06-21T18:41:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n\n> > Fair enough, here's a version I think you and others will find\n> > acceptable then. It allows users like me who like this to explicitly\n> > opt-in via color.man=true.\n> \n> Not really.\n> \n> Since the implementation of the posted patch, as I understand it,\n> does not aim to affect both \"git help -m foo\" and \"man git-foo\"\n> identically,\n\nIt cannot aim to do what is not possible.\n\n> I think it would be easier to understand to end-users\n> if this were exposed as a new \"mode\", like \"git help --web\" and \"git\n> help --info\" are different modes from the \"git help --man\",\n> something like \"git help --fancy-man\" (or whatever is easy to type\n> and explain, and also add it to the variants help.format knows about\n> to make it easy to set the default).\n\nBut it is not a new mode.\n\nI presume you mean format, since man, info, and web are formats,\ncontrolled by help.format.\n\nBut no, it's not a format either, because we still want to see the same\nformat (man).\n\nPerhaps you meant a man viewer (controlled with man.viewer), but there's\nno command line option to launch help with for example emacs woman.\n\nBut still, it's not a new viewer; it's an improvement of an already\nexisting viewer.\n\n> One advantage of doing so is that we do not have to worry about \"ah,\n> user has LESS_BLAH environment variable so we should disable this\n> new mode here\" etc.\n\nWe don't have to worry about that with the current patch.\n\n> As long as the new mode is requested either via\n> the command line option or help.format configuration, it can\n> completely take it over.\n\nThat is already the case.\n\n> That simplifies the necessary explanation given to the users quite a\n> lot, no?\n\nNo, it would still be:\n\n 1. man.viewer=fancyman must be set in the config\n 2. The user must use less\n 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n 4. Have color.ui enabled\n 5. Not have color.pager disabled\n 6. Not have git with stdout directed to a file\n\nMoreover, this explanation is for developers.\n\nRealistically all the user needs to know is that color.man=true turns\nthis on (man.viewer=fancyman is not better in any way).\n\n-- \nFelipe Contreras"},{"id":"428093","messageId":"87mtrj2faq.fsf@evledraar.gmail.com","threadId":"55761","inReplyTo":"xmqqfsxbika3.fsf@gitster.g","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-21T19:08:20Z","receivedAt":"2021-06-21T19:23:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 21 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> So, in order for this change to have any effect:\n>>\n>> 1. color.man=true must be set in the config\n>> 2. The user must use less\n>> 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n>> 4. Have color.ui enabled\n>> 5. Not have color.pager disabled\n>> 6. Not have git with stdout directed to a file\n>>\n>> 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n>> 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n>>\n>> Suggested-by: Ævar Arnfjörð Bjarmason \n>> Signed-off-by: Felipe Contreras \n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>>\n>> On Tue, Jun 08 2021, Junio C Hamano wrote:\n>>\n>>> Ævar Arnfjörð Bjarmason writes:\n>>>\n>>>> I've been running with this on my personal git build since May 26th. I\n>>>> haven't had any issues with it, and I like the new coloring.\n>>>> ...\n>>>> I think this is a good example of a change that we're better off just\n>>>> merging down and then reverting if the wider audience of git users hates\n>>>> it, rather than trying to come to some perfect consensus here\n>>>> on-list.\n>>>\n>>> My impression was tht we already had a rough consensus here on-list\n>>> that it may be good to educate users who like this \"new coloring\"\n>>> like you do to configure their \"less\", so that they consistently get\n>>> the \"new coloring\" they like whether they are doing \"git help git\",\n>>> \"man git\", or even \"man ls\", and the approach the posted patch takes\n>>> will not help (it only affects \"git help git\" among these).\n>>>\n>>> I'd rather not to take it.\n>>\n>> Fair enough, here's a version I think you and others will find\n>> acceptable then. It allows users like me who like this to explicitly\n>> opt-in via color.man=true.\n>\n> Not really.\n>\n> [snip] I think it would be easier to understand to end-users\n> if this were exposed as a new \"mode\", like \"git help --web\" and \"git\n> help --info\" are different modes from the \"git help --man\",\n> something like \"git help --fancy-man\" (or whatever is easy to type\n> and explain, and also add it to the variants help.format knows about\n> to make it easy to set the default).\n>\n> One advantage of doing so is that we do not have to worry about \"ah,\n> user has LESS_BLAH environment variable so we should disable this\n> new mode here\" etc. As long as the new mode is requested either via\n> the command line option or help.format configuration, it can\n> completely take it over. That simplifies the necessary explanation\n> given to the users quite a lot, no?\n\nThe interaction between \"git help\" and \"man\"/\"less\" doesn't really have\nan equivalent in the rest of git as far as color output goes. Usually we\nemit colors via our own programs.\n\nBut no, I think it makes the most sense to consider this orthagonal to\nhelp.format=man or man.viewer=.\n\nWe're not invoking a different man viewer or command, we're just\nexpecting that mode to invoke the pager, and if that pager is less to\nhave these variables tweak our color preferences.\n\n> [unsnip] Since the implementation of the posted patch, as I understand it,\n> does not aim to affect both \"git help -m foo\" and \"man git-foo\"\n> identically, \n\nAside from this patch I don't think it makes sense to view git's UI and\ninteraction with the pager like that.\n\nTo probably >95% of our users the \"we invoke the pager\" is just a\ntechnical implementation details they're not aware of. Git just has\npretty colors by default, so I don't think it's jarring that \"git help\nxyz\" and \"man git-xyz\" look different.\n\nWe also don't try to maintain the UI that:\n\n git cmd >file &&\n pager file\n\nGives you the same UX as:\n\n # Invokes pager(1) for you\n git cmd\n\nSince we set e.g. PAGER_ENV already, I believe this was brought up in\npast discussions. So we're already past the point of git adding its own\nmagic custom options to feed to the pager.\n\nSo I don't see the problem with having \"a bit more like PAGER_ENV\"\nhidden behind a color.man=true config option in this case.\n\n>> \t----\n>> \tgit-config - Get and set [...]\n>>\n>> \tSYNOPSIS\n>> \t--------\n>> \t[...]\n>> \t'git config' [] [...]\n>> \t[...]\n>> \tThe `--type=` option instructs 'git config' to ensure [...]\n>>\n>> Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n>> config\" and other '-quoted parts in BLUE UNDERLINE instead of\n>> UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n>> instead of BOLD. The \"Standout\" setting is then used for the user's\n>> own search bar (invoked with \"/\") and prompt. See [2] for more\n>> examples\n>\n> There are BOLD RED and READ BOLD; are they differently rendered?\n\nYes, that's just an omission/mistake. It should be the \"RED BOLD\",\ni.e. \" \".\n"},{"id":"428347","messageId":"YNPKwIuZvpyWSNXH@coredump.intra.peff.net","threadId":"55761","inReplyTo":"87mtrj2faq.fsf@evledraar.gmail.com","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-23T23:58:56Z","receivedAt":"2021-06-23T23:58:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 21, 2021 at 09:08:20PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > [snip] I think it would be easier to understand to end-users\n> > if this were exposed as a new \"mode\", like \"git help --web\" and \"git\n> > help --info\" are different modes from the \"git help --man\",\n> > something like \"git help --fancy-man\" (or whatever is easy to type\n> > and explain, and also add it to the variants help.format knows about\n> > to make it easy to set the default).\n> >\n> > One advantage of doing so is that we do not have to worry about \"ah,\n> > user has LESS_BLAH environment variable so we should disable this\n> > new mode here\" etc. As long as the new mode is requested either via\n> > the command line option or help.format configuration, it can\n> > completely take it over. That simplifies the necessary explanation\n> > given to the users quite a lot, no?\n> \n> The interaction between \"git help\" and \"man\"/\"less\" doesn't really have\n> an equivalent in the rest of git as far as color output goes. Usually we\n> emit colors via our own programs.\n> \n> But no, I think it makes the most sense to consider this orthagonal to\n> help.format=man or man.viewer=.\n> \n> We're not invoking a different man viewer or command, we're just\n> expecting that mode to invoke the pager, and if that pager is less to\n> have these variables tweak our color preferences.\n\nFWIW, if we are going to do this, then just having it as \"color.man\"\nmakes the most sense to me. It is easily explained as \"when we invoke\nman, set up some environment variables that may enable colors in the\noutput\".\n\nI'm still entirely unconvinced that this should be in Git at all;\npointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like\nit would be sufficient. But it feels like that conversation was not\ngoing anywhere productive; I mention it here only to indicate that my\nresponse above is not an endorsement of the concept.\n\n-Peff\n"},{"id":"428348","messageId":"YNPM4yvk/71oeAPx@coredump.intra.peff.net","threadId":"55761","inReplyTo":"patch-1.1-a950ef49e28-20210621T083254Z-avarab@gmail.com","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-24T00:08:03Z","receivedAt":"2021-06-24T00:08:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 21, 2021 at 10:34:00AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\n> index e05d520a867..2f12ae3386d 100644\n> --- a/Documentation/config/color.txt\n> +++ b/Documentation/config/color.txt\n> @@ -126,6 +126,12 @@ color.interactive.::\n> \tor `error`, for four distinct types of normal output from\n> \tinteractive commands.\n> \n> +color.man::\n> +\tThis flag can be used to enable the automatic colorizaton of man\n> +\tpages when using the less pager, `false` by default. When set to\n> +\t`true` it's activated only when `color.ui` allows it, and if\n> +\t`color.pager` enable (which it is by default).\n\nA few typos here:\n\ndiff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\nindex 2f12ae3386..fcc12df508 100644\n--- a/Documentation/config/color.txt\n+++ b/Documentation/config/color.txt\n@@ -127,10 +127,10 @@ color.interactive.::\n \tinteractive commands.\n \n color.man::\n-\tThis flag can be used to enable the automatic colorizaton of man\n+\tThis flag can be used to enable the automatic colorization of man\n \tpages when using the less pager, `false` by default. When set to\n \t`true` it's activated only when `color.ui` allows it, and if\n-\t`color.pager` enable (which it is by default).\n+\t`color.pager` is enabled (which it is by default).\n \n color.pager::\n \tA boolean to specify whether `auto` color modes should colorize\n\nThe interaction with color.ui seems unusual. Normally it is not a\ngate-keeper for specific colorizations, but rather a fallback when\nmore-specific color config is unspecified. E.g.:\n\n [color]\n ui = false\n branch = true\n\nwould colorize branch output, but nothing else. But from your\ndescription (and I think the code matches this), doing:\n\n [color]\n ui = false\n man = true\n\nwould still disable the man-colors. So there's no way to enable this\nfeature without enabling colors everywhere else. I think it should\nsimply be independent of color.ui (with the exception that it may\neventually use it as a fallback like all the other color.* booleans,\n_if_ we want to move it to default-to-on).\n\n-Peff\n"},{"id":"428353","messageId":"60d3d9cf77ddb_b4120889@natae.notmuch","threadId":"55761","inReplyTo":"YNPKwIuZvpyWSNXH@coredump.intra.peff.net","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-24T01:03:11Z","receivedAt":"2021-06-24T01:03:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Mon, Jun 21, 2021 at 09:08:20PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> > > [snip] I think it would be easier to understand to end-users\n> > > if this were exposed as a new \"mode\", like \"git help --web\" and \"git\n> > > help --info\" are different modes from the \"git help --man\",\n> > > something like \"git help --fancy-man\" (or whatever is easy to type\n> > > and explain, and also add it to the variants help.format knows about\n> > > to make it easy to set the default).\n> > >\n> > > One advantage of doing so is that we do not have to worry about \"ah,\n> > > user has LESS_BLAH environment variable so we should disable this\n> > > new mode here\" etc. As long as the new mode is requested either via\n> > > the command line option or help.format configuration, it can\n> > > completely take it over. That simplifies the necessary explanation\n> > > given to the users quite a lot, no?\n> > \n> > The interaction between \"git help\" and \"man\"/\"less\" doesn't really have\n> > an equivalent in the rest of git as far as color output goes. Usually we\n> > emit colors via our own programs.\n> > \n> > But no, I think it makes the most sense to consider this orthagonal to\n> > help.format=man or man.viewer=.\n> > \n> > We're not invoking a different man viewer or command, we're just\n> > expecting that mode to invoke the pager, and if that pager is less to\n> > have these variables tweak our color preferences.\n> \n> FWIW, if we are going to do this, then just having it as \"color.man\"\n> makes the most sense to me. It is easily explained as \"when we invoke\n> man, set up some environment variables that may enable colors in the\n> output\".\n> \n> I'm still entirely unconvinced that this should be in Git at all;\n\nThat's OK, you don't need to be convinced for this change to be a\npositive one.\n\n> pointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like\n> it would be sufficient.\n\nWhat color-man wrapper?\n\n> But it feels like that conversation was not going anywhere productive;\n\nFeelings are not facts.\n\nBailing from a discussion doesn't resolve the discussion, and the\nquestion \"how is a user supposed to configure this properly?\" remains\nunanswered by you, or anyone [1].\n\nThis patch is the closest to a convenient solution anybody has come up\nwith.\n\nIf anybody has any other proposal it would be good to hear them.\n\n[1] https://lore.kernel.org/git/60bfadc0aca09_1abb8f208fd@natae.notmuch/\n\n-- \nFelipe Contreras"},{"id":"428354","messageId":"60d3e399945d_2cc520855@natae.notmuch","threadId":"55761","inReplyTo":"20210523054454.1188757-1-felipe.contreras@gmail.com","subject":"RE: [PATCH v6] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-24T01:44:57Z","receivedAt":"2021-06-24T01:47:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"> --- a/builtin/help.c\n> +++ b/builtin/help.c\n\n> @@ -264,6 +289,7 @@ static void exec_man_man(const char *path, const char *page)\n> static void exec_man_cmd(const char *cmd, const char *page)\n> {\n> \tstruct strbuf shell_cmd = STRBUF_INIT;\n> +\tcolorize_man();\n\nOn further reflection I don't think this colorize belongs here.\nexec_man_cmd() is meant to execute any custom command, not necessarily\nman.\n\n-- \nFelipe Contreras\n"},{"id":"428511","messageId":"20210626025040.104428-1-felipe.contreras@gmail.com","threadId":"55761","inReplyTo":"20210523054454.1188757-1-felipe.contreras@gmail.com","subject":"[PATCH v8] help: add option to colorize man pages under less","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-26T02:50:40Z","receivedAt":"2021-06-26T02:50:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"We already colorize tools traditionally not colorized by default--like\ndiff and grep. Let's do the same for man, but only if `color.man` is\nexplicitly set to \"true\".\n\nUnlike other `color.*` output, this colorization is not enabled when\n`color.ui` is true; the user needs to explicitly set the\n`color.man` variable to `true.\n\nWhen it was proposed to treat `color.man` like any other `color.*`\nvariable, some thought that git opting to add color for an external\nprogram such as man(1) was a step too far [1]--even if the user invoked\nit via the \"git help \" wrapper. So let's make this explicitly\nopt-in for now.\n\nAs noted in the documentation we're leaving ourselves an out to turn\nthis on by default in the future, for example putting it under the\nfeature.experimental umbrella. We probably won't, but let's not promise\nusers that `color.man` will forever be a special-case.\n\nAs for what this actually does, the effect of having this enabled is\nthat a documentation blurb like (some parts elided with \"[...]\"):\n\n\tNAME\n\t----\n\tgit-config - Get and set [...]\n\n\tSYNOPSIS\n\t--------\n\t[...]\n\t'git config' [] [...]\n\t[...]\n\tThe `--type=` option instructs 'git config' to ensure [...]\n\nWill have \"NAME\" and \"SYNOPSIS\" shown as RED BOLD instead of BOLD,\n\"git config\" and other '-quoted parts in BLUE UNDERLINE instead of\nUNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\ninstead of BOLD. The \"standout\" setting is then used for the user's\nown search bar (invoked with \"/\") and prompt. See [2] for more\nexamples\n\nNormally check_auto_color() would check the value of `color.pager`, but\nin this particular case it's not git the one executing the pager, but\nman. Therefore we need to check pager_use_color ourselves.\n\nWe do not need to support `color.man` being set to `always`; the `git\nhelp` command is always run for a tty (it would be very strange for a\nuser to do `git help $page > output`, but in fact, that works anyway,\nwe don't even need to check if stdout is a tty, but just to be\nconsistent we do). So it's simply a boolean in our case.\n\nSo, in order for this change to have any effect:\n\n 1. color.man=true must be set in the config\n 2. The user must use less\n 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n 4. Not have color.pager disabled\n 5. Not have git with stdout directed to a file\n\n1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n\nSuggested-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Felipe Contreras \n---\n\nThis a reroll of Ævar's v7 with comments from Jeff King so that\ncolor.ui=never doesn't disable color.man, and the documentation was\nupdated accordingly.\n\nAdditinally I removed one call to colorize_man() in exec_man_cmd() which\nis not meant for the man command (although it could be used for that).\n\nPlus a bunch of style changes to the commit message.\n\nRange-diff against v7:\n1: 167f5c8b39 ! 1: 1b3f7ee1aa help: colorize man pages if man.color=true under less(1)\n @@ Metadata\n Author: Felipe Contreras \n \n ## Commit message ##\n - help: colorize man pages if man.color=true under less(1)\n + help: add option to colorize man pages under less\n \n - We already colorize tools traditionally not colorized by default, like\n + We already colorize tools traditionally not colorized by default--like\n diff and grep. Let's do the same for man, but only if `color.man` is\n explicitly set to \"true\".\n \n - Unlike other `color.*` output this colorization is not enabled by\n - `color.ui` being true, the user needs to explicitly set the\n + Unlike other `color.*` output, this colorization is not enabled when\n + `color.ui` is true; the user needs to explicitly set the\n `color.man` variable to `true.\n \n When it was proposed to treat `color.man` like any other `color.*`\n - variable some thought that git opting in coloring for an external\n - program such as man(1) was a step too far[1], even if the user invoked\n - it via the \"git help \" wrapper.\n + variable, some thought that git opting to add color for an external\n + program such as man(1) was a step too far [1]--even if the user invoked\n + it via the \"git help \" wrapper. So let's make this explicitly\n + opt-in for now.\n \n - So let's make this explicitly opt-in for now. As noted in the\n - documentation we're leaving ourselves an out to turn this on by\n - default in the future, or e.g. putting it under the\n - feature.experimental umbrella. We probably won't, but let's not\n - promise users that `color.man` will forever be a special-case.\n + As noted in the documentation we're leaving ourselves an out to turn\n + this on by default in the future, for example putting it under the\n + feature.experimental umbrella. We probably won't, but let's not promise\n + users that `color.man` will forever be a special-case.\n \n - As for what this actually does the effect of having this enabled is\n + As for what this actually does, the effect of having this enabled is\n that a documentation blurb like (some parts elided with \"[...]\"):\n \n NAME\n @@ Commit message\n [...]\n The `--type=` option instructs 'git config' to ensure [...]\n \n - Will have \"NAME\" and \"SECTION\" shown as BOLD RED instead of BOLD, \"git\n - config\" and other '-quoted parts in BLUE UNDERLINE instead of\n + Will have \"NAME\" and \"SYNOPSIS\" shown as RED BOLD instead of BOLD,\n + \"git config\" and other '-quoted parts in BLUE UNDERLINE instead of\n UNDERLINE, and `--type=` and other `-quoted parts in RED BOLD\n - instead of BOLD. The \"Standout\" setting is then used for the user's\n + instead of BOLD. The \"standout\" setting is then used for the user's\n own search bar (invoked with \"/\") and prompt. See [2] for more\n examples\n \n @@ Commit message\n in this particular case it's not git the one executing the pager, but\n man. Therefore we need to check pager_use_color ourselves.\n \n - We do not need to support `color.man` being set to `always`; The `git\n + We do not need to support `color.man` being set to `always`; the `git\n help` command is always run for a tty (it would be very strange for a\n user to do `git help $page > output`, but in fact, that works anyway,\n we don't even need to check if stdout is a tty, but just to be\n @@ Commit message\n 1. color.man=true must be set in the config\n 2. The user must use less\n 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n - 4. Have color.ui enabled\n - 5. Not have color.pager disabled\n - 6. Not have git with stdout directed to a file\n + 4. Not have color.pager disabled\n + 5. Not have git with stdout directed to a file\n \n 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/\n 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147\n \n Suggested-by: Ævar Arnfjörð Bjarmason \n - Signed-off-by: Felipe Contreras \n Signed-off-by: Ævar Arnfjörð Bjarmason \n + Signed-off-by: Felipe Contreras \n \n ## Documentation/config/color.txt ##\n @@ Documentation/config/color.txt: color.interactive.::\n @@ Documentation/config/color.txt: color.interactive.::\n \tinteractive commands.\n \n +color.man::\n -+\tThis flag can be used to enable the automatic colorizaton of man\n ++\tThis flag can be used to enable the automatic colorization of man\n +\tpages when using the less pager, `false` by default. When set to\n -+\t`true` it's activated only when `color.ui` allows it, and if\n -+\t`color.pager` enable (which it is by default).\n ++\t`true` it's activated only if `color.pager` is enabled (which it\n ++\tis by default).\n +\n color.pager::\n \tA boolean to specify whether `auto` color modes should colorize\n @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag\n \n +static void colorize_man(void)\n +{\n -+\tif (!man_color || !want_color(GIT_COLOR_UNKNOWN) || !pager_use_color)\n ++\tif (!man_color || !pager_use_color)\n +\t\treturn;\n +\n +\t/* Disable groff colors */\n @@ builtin/help.c: static void exec_man_konqueror(const char *path, const char *pag\n \texeclp(path, \"man\", page, (char *)NULL);\n \twarning_errno(_(\"failed to exec '%s'\"), path);\n }\n -@@ builtin/help.c: static void exec_man_man(const char *path, const char *page)\n - static void exec_man_cmd(const char *cmd, const char *page)\n - {\n - \tstruct strbuf shell_cmd = STRBUF_INIT;\n -+\tcolorize_man();\n - \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n - \texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n - \twarning(_(\"failed to exec '%s'\"), cmd);\n @@ builtin/help.c: static int git_help_config(const char *var, const char *value, void *cb)\n \t}\n \tif (starts_with(var, \"man.\"))\n\n Documentation/config/color.txt | 11 +++++++++++\n builtin/help.c | 31 ++++++++++++++++++++++++++++++-\n color.h | 1 +\n 3 files changed, 42 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/color.txt b/Documentation/config/color.txt\nindex e05d520a86..8511af1f1d 100644\n--- a/Documentation/config/color.txt\n+++ b/Documentation/config/color.txt\n@@ -126,6 +126,12 @@ color.interactive.::\n \tor `error`, for four distinct types of normal output from\n \tinteractive commands.\n \n+color.man::\n+\tThis flag can be used to enable the automatic colorization of man\n+\tpages when using the less pager, `false` by default. When set to\n+\t`true` it's activated only if `color.pager` is enabled (which it\n+\tis by default).\n+\n color.pager::\n \tA boolean to specify whether `auto` color modes should colorize\n \toutput going to the pager. Defaults to true; set this to false\n@@ -200,3 +206,8 @@ color.ui::\n \toutput not intended for machine consumption to use color, to\n \t`true` or `auto` (this is the default since Git 1.8.4) if you\n \twant such output to use color when written to the terminal.\n++\n+When set to `true` certain other `color.*` variables may still not be\n+turned on unless explicitly enabled. Currently this only applies to\n+`color.man`, see above. Such opt-in variables may be moved under the\n+default `color.ui` umbrella in the future.\ndiff --git a/builtin/help.c b/builtin/help.c\nindex bb339f0fc8..150dd05fdb 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -11,6 +11,7 @@\n #include \"config-list.h\"\n #include \"help.h\"\n #include \"alias.h\"\n+#include \"color.h\"\n \n #ifndef DEFAULT_HELP_FORMAT\n #define DEFAULT_HELP_FORMAT \"man\"\n@@ -34,6 +35,7 @@ enum help_format {\n \tHELP_FORMAT_WEB\n };\n \n+static int man_color;\n static const char *html_path;\n \n static int show_all = 0;\n@@ -253,10 +255,33 @@ static void exec_man_konqueror(const char *path, const char *page)\n \t}\n }\n \n+static void colorize_man(void)\n+{\n+\tif (!man_color || !pager_use_color)\n+\t\treturn;\n+\n+\t/* Disable groff colors */\n+\tsetenv(\"GROFF_NO_SGR\", \"1\", 0);\n+\n+\t/* Bold */\n+\tsetenv(\"LESS_TERMCAP_md\", GIT_COLOR_BOLD_RED, 0);\n+\tsetenv(\"LESS_TERMCAP_me\", GIT_COLOR_RESET, 0);\n+\n+\t/* Underline */\n+\tsetenv(\"LESS_TERMCAP_us\", GIT_COLOR_BLUE GIT_COLOR_UNDERLINE, 0);\n+\tsetenv(\"LESS_TERMCAP_ue\", GIT_COLOR_RESET, 0);\n+\n+\t/* Standout */\n+\tsetenv(\"LESS_TERMCAP_so\", GIT_COLOR_CYAN GIT_COLOR_REVERSE, 0);\n+\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\n+}\n+\n static void exec_man_man(const char *path, const char *page)\n {\n \tif (!path)\n \t\tpath = \"man\";\n+\n+\tcolorize_man();\n \texeclp(path, \"man\", page, (char *)NULL);\n \twarning_errno(_(\"failed to exec '%s'\"), path);\n }\n@@ -371,8 +396,12 @@ static int git_help_config(const char *var, const char *value, void *cb)\n \t}\n \tif (starts_with(var, \"man.\"))\n \t\treturn add_man_viewer_info(var, value);\n+\tif (!strcmp(var, \"color.man\")) {\n+\t\tman_color = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n-\treturn git_default_config(var, value, cb);\n+\treturn git_color_default_config(var, value, cb);\n }\n \n static struct cmdnames main_cmds, other_cmds;\ndiff --git a/color.h b/color.h\nindex 98894d6a17..d012add4e8 100644\n--- a/color.h\n+++ b/color.h\n@@ -51,6 +51,7 @@ struct strbuf;\n #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n+#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n \n /* A special value meaning \"no color selected\" */\n #define GIT_COLOR_NIL \"NIL\"\n-- \n2.32.0\n\n"},{"id":"428521","messageId":"877digy8xe.fsf@evledraar.gmail.com","threadId":"55761","inReplyTo":"20210626025040.104428-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v8] help: add option to colorize man pages under less","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-26T14:49:46Z","receivedAt":"2021-06-26T14:53:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jun 25 2021, Felipe Contreras wrote:\n\n> This a reroll of Ævar's v7 with comments from Jeff King so that\n> color.ui=never doesn't disable color.man, and the documentation was\n> updated accordingly.\n>\n> Additinally I removed one call to colorize_man() in exec_man_cmd() which\n> is not meant for the man command (although it could be used for that).\n>\n> Plus a bunch of style changes to the commit message.\n\nThis version looks good to me, thanks for the style & grammar fixes on\nmy commit message additions.\n\n> 1. color.man=true must be set in the config\n> 2. The user must use less\n> 3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)\n> - 4. Have color.ui enabled\n> - 5. Not have color.pager disabled\n> - 6. Not have git with stdout directed to a file\n> + 4. Not have color.pager disabled\n> + 5. Not have git with stdout directed to a file\n\nEven better.\n\n> static struct cmdnames main_cmds, other_cmds;\n> diff --git a/color.h b/color.h\n> index 98894d6a17..d012add4e8 100644\n> --- a/color.h\n> +++ b/color.h\n> @@ -51,6 +51,7 @@ struct strbuf;\n> #define GIT_COLOR_FAINT\t\t\"\\033[2m\"\n> #define GIT_COLOR_FAINT_ITALIC\t\"\\033[2;3m\"\n> #define GIT_COLOR_REVERSE\t\"\\033[7m\"\n> +#define GIT_COLOR_UNDERLINE\t\"\\033[4m\"\n> \n> /* A special value meaning \"no color selected\" */\n> #define GIT_COLOR_NIL \"NIL\"\n\nNot really needing, but I note that this adds something that we don't\nhave in test_decode_color(), not that we're testing this output\ndirectly, so it doesn't matter for now.\n"},{"id":"428616","messageId":"xmqqlf6t7uvh.fsf@gitster.g","threadId":"55761","inReplyTo":"YNPKwIuZvpyWSNXH@coredump.intra.peff.net","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-28T17:37:54Z","receivedAt":"2021-06-28T17:37:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm still entirely unconvinced that this should be in Git at all;\n> pointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like\n> it would be sufficient. But it feels like that conversation was not\n> going anywhere productive; I mention it here only to indicate that my\n> response above is not an endorsement of the concept.\n\nI have the same reaction to the patch.\n"},{"id":"428618","messageId":"60da0f7742d73_1b8dc20826@natae.notmuch","threadId":"55761","inReplyTo":"xmqqlf6t7uvh.fsf@gitster.g","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-28T18:05:43Z","receivedAt":"2021-06-28T18:05:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > I'm still entirely unconvinced that this should be in Git at all;\n> > pointing GIT_MAN_VIEWER or man.*.cmd at a color-man wrapper seems like\n> > it would be sufficient. But it feels like that conversation was not\n> > going anywhere productive; I mention it here only to indicate that my\n> > response above is not an endorsement of the concept.\n> \n> I have the same reaction to the patch.\n\nDo you care to explain why you think so?\n\nWhat is the alternative in your view? What configuration should users\nuse instead?\n\n-- \nFelipe Contreras\n"},{"id":"428662","messageId":"xmqq7did30q6.fsf@gitster.g","threadId":"55761","inReplyTo":"YNPM4yvk/71oeAPx@coredump.intra.peff.net","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-29T01:42:41Z","receivedAt":"2021-06-29T01:42:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> .... But from your\n> description (and I think the code matches this), doing:\n>\n> [color]\n> ui = false\n> man = true\n>\n> would still disable the man-colors. So there's no way to enable this\n> feature without enabling colors everywhere else. I think it should\n> simply be independent of color.ui (with the exception that it may\n> eventually use it as a fallback like all the other color.* booleans,\n> _if_ we want to move it to default-to-on).\n\nThat matches my perception. Thanks.\n"},{"id":"428663","messageId":"60da7be142144_2054b2088d@natae.notmuch","threadId":"55761","inReplyTo":"xmqq7did30q6.fsf@gitster.g","subject":"Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-29T01:48:17Z","receivedAt":"2021-06-29T01:48:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > .... But from your\n> > description (and I think the code matches this), doing:\n> >\n> > [color]\n> > ui = false\n> > man = true\n> >\n> > would still disable the man-colors. So there's no way to enable this\n> > feature without enabling colors everywhere else. I think it should\n> > simply be independent of color.ui (with the exception that it may\n> > eventually use it as a fallback like all the other color.* booleans,\n> > _if_ we want to move it to default-to-on).\n> \n> That matches my perception. Thanks.\n\nThis is already integrated into v8.\n\nhttps://lore.kernel.org/git/20210626025040.104428-1-felipe.contreras@gmail.com/\n\n-- \nFelipe Contreras\n"}]}