{"thread":{"id":"55742","subject":"[PATCH v4] help: colorize man pages","startedAt":"2021-05-20T04:07:35Z","lastAt":"2021-05-22T20:53:49Z","messageCount":18,"participants":["Felipe Contreras","Phillip Wood","Leah Neukirchen","Junio C Hamano","Jeff King","Philip Oakley"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"425030","messageId":"20210520040725.133848-1-felipe.contreras@gmail.com","threadId":"55742","inReplyTo":null,"subject":"[PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-20T04:07:25Z","receivedAt":"2021-05-20T04:07:35Z","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 the LESS variable to render bold, underlined, and standout\ntext with colors in the less pager.\n\nBold is rendered as red, underlined as blue, and standout (prompt and\nhighlighted search) as inverse magenta.\n\nObviously this only works when the less pager is used.\n\nIf the user has already set the LESS variable in his/her environment,\nthat is 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 automatically [probably thanks to less being smart], we don't even\nneed to check if stdout is a tty, but just to be consistent we do). So\nit's simply a boolean in our case.\n\nMoreover, just to be painstakingly comprehensive with people who have\ncolor-aversion; we honour NO_COLOR [1].\n\nSo, 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 3. Have color.ui enabled\n 4. Not have color.pager disabled\n 5. Not have color.man disabled\n 6. Not have NO_COLOR set\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://no-color.org/\n\nSuggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nComments-by: Jeff King <peff@peff.net>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\nRange-diff against v3:\n1:  db93bf432b ! 1:  7249785014 help: colorize man pages\n    @@ Metadata\n      ## Commit message ##\n         help: colorize man pages\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    +    We can set the LESS variable to render bold, underlined, and standout\n    +    text with colors in the less pager.\n     \n    -    Bold is rendered as red, underlined as blue, and standout (messages and\n    +    Bold is rendered as red, underlined as blue, and standout (prompt and\n         highlighted search) as inverse magenta.\n     \n    -    This only works when the pager is less.\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 not overwritten.\n    +    If the user has already set the LESS variable in his/her environment,\n    +    that is 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, and in\n    -    addition color.pager needs to be turned on.\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; git help is always run for a tty (it would be very\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 automatically [probably thanks to less being smart], we don't even\n         need to check if stdout is a tty, but just to be consistent we do). So\n         it's simply a boolean in our case.\n     \n    -    So in order for this to have an effect:\n    +    Moreover, just to be painstakingly comprehensive with people who have\n    +    color-aversion; we honour NO_COLOR [1].\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    +     2. Not have the LESS variable set\n          3. Have color.ui enabled\n    -     4. Have color.pager enabled\n    +     4. Not have color.pager disabled\n          5. Not have color.man disabled\n    -     6. Run git with stdout on a tty\n    +     6. Not have NO_COLOR set\n    +     7. Not have git with stdout directed to a file\n     \n    -    Otherwise the current behavior remains.\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    +    [1] https://no-color.org/\n     \n         Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n         Comments-by: Jeff King <peff@peff.net>\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/* See: https://no-color.org/ */\n    ++\tif (getenv(\"NO_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_MAGENTA GIT_COLOR_REVERSE, 0);\n    -+\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\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\", 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                 | 28 +++++++++++++++++++++++++++-\n 2 files changed, 32 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.<slot>::\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..298d97cc39 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,29 @@ 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/* See: https://no-color.org/ */\n+\tif (getenv(\"NO_COLOR\"))\n+\t\treturn;\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\", 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 +285,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 +393,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-- \n2.31.1\n\n"},{"id":"425076","messageId":"842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com","threadId":"55742","inReplyTo":"20210520040725.133848-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-05-20T09:26:03Z","receivedAt":"2021-05-20T09:27:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/05/2021 05:07, 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\nI think there is a distinction between 'diff' and 'grep' where we are \ngenerating the content and help where we are running man - I would \nexpect a man page to look the same whether it is displayed by 'man git \nfoo' or 'git help foo'\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 the LESS variable to render bold, underlined, and standout\n> text with colors in the less pager.\n> \n> Bold is rendered as red, underlined as blue, and standout (prompt and\n> highlighted search) as inverse magenta.\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.\n\nHowever if they have specified the colors they would like by using the \nLESS_TERMCAP_xx environment variables that the previous versions of this \npatch used their choice is overridden by this new patch.\n\nI've got LESS_TERMCAP_xx set and running\n\tLESS='Dd+r$Du+b$Ds' man git add\nchanges the output colors\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\ns/git the one/git is not/\n\nBest Wishes\n\nPhillip\n\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 automatically [probably thanks to less being smart], we don't even\n> need to check if stdout is a tty, but just to be consistent we do). So\n> it's simply a boolean in our case.\n> \n> Moreover, just to be painstakingly comprehensive with people who have\n> color-aversion; we honour NO_COLOR [1].\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 LESS variable set\n>   3. Have color.ui enabled\n>   4. Not have color.pager disabled\n>   5. Not have color.man disabled\n>   6. Not have NO_COLOR set\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://no-color.org/\n> \n> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> Comments-by: Jeff King <peff@peff.net>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n> Range-diff against v3:\n> 1:  db93bf432b ! 1:  7249785014 help: colorize man pages\n>      @@ Metadata\n>        ## Commit message ##\n>           help: colorize man pages\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>      +    We can set the LESS variable to render bold, underlined, and standout\n>      +    text with colors in the less pager.\n>       \n>      -    Bold is rendered as red, underlined as blue, and standout (messages and\n>      +    Bold is rendered as red, underlined as blue, and standout (prompt and\n>           highlighted search) as inverse magenta.\n>       \n>      -    This only works when the pager is less.\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 not overwritten.\n>      +    If the user has already set the LESS variable in his/her environment,\n>      +    that is 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, and in\n>      -    addition color.pager needs to be turned on.\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; git help is always run for a tty (it would be very\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 automatically [probably thanks to less being smart], we don't even\n>           need to check if stdout is a tty, but just to be consistent we do). So\n>           it's simply a boolean in our case.\n>       \n>      -    So in order for this to have an effect:\n>      +    Moreover, just to be painstakingly comprehensive with people who have\n>      +    color-aversion; we honour NO_COLOR [1].\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>      +     2. Not have the LESS variable set\n>            3. Have color.ui enabled\n>      -     4. Have color.pager enabled\n>      +     4. Not have color.pager disabled\n>            5. Not have color.man disabled\n>      -     6. Run git with stdout on a tty\n>      +     6. Not have NO_COLOR set\n>      +     7. Not have git with stdout directed to a file\n>       \n>      -    Otherwise the current behavior remains.\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>      +    [1] https://no-color.org/\n>       \n>           Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>           Comments-by: Jeff King <peff@peff.net>\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/* See: https://no-color.org/ */\n>      ++\tif (getenv(\"NO_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_MAGENTA GIT_COLOR_REVERSE, 0);\n>      -+\tsetenv(\"LESS_TERMCAP_se\", GIT_COLOR_RESET, 0);\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\", 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                 | 28 +++++++++++++++++++++++++++-\n>   2 files changed, 32 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.<slot>::\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..298d97cc39 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,29 @@ 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/* See: https://no-color.org/ */\n> +\tif (getenv(\"NO_COLOR\"))\n> +\t\treturn;\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\", 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 +285,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 +393,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> \n"},{"id":"425103","messageId":"60a66b11d6ffd_2448320885@natae.notmuch","threadId":"55742","inReplyTo":"842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-20T13:58:41Z","receivedAt":"2021-05-20T13:59:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 20/05/2021 05:07, 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> I think there is a distinction between 'diff' and 'grep' where we are \n> generating the content and help where we are running man\n\nIt makes a difference for git developers, not for the user.\n\nThe user doesn't care how the output of `git grep` was generated, all\nshe sees is that it's different from `grep`. It's in fact more\nsurprising than a difference in `git help` because it's even the same\ncomand.\n\nMaybe if the command was `git man` they would be equally surprising, but\nit's not, in fact, `git help` can be used to 1) output directly to the\nterminal 2) view in a browser, 3) view in info program, 4) view man page\nin woman, 5) view the man page in koqueror 6) view the man page in man.\n\nOnly in one case among many would the user expect to see man, therefore\na colorized `git grep` is more surprising.\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 the LESS variable to render bold, underlined, and standout\n> > text with colors in the less pager.\n> > \n> > Bold is rendered as red, underlined as blue, and standout (prompt and\n> > highlighted search) as inverse magenta.\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.\n> \n> However if they have specified the colors they would like by using the \n> LESS_TERMCAP_xx environment variables that the previous versions of this \n> patch used their choice is overridden by this new patch.\n\nThat is true. We could add a check for that:\n\n  if (getenv(\"LESS_TERMCAP_md\"))\n          return;\n\nHowever, it may not be necessary since many of the tips online set these\nvariables inside a function.\n\n> I've got LESS_TERMCAP_xx set and running\n> \tLESS='Dd+r$Du+b$Ds' man git add\n> changes the output colors\n\nYou have them set in the environtment? Not inside a function like\nman () { ... command man \"$@\" } ?\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> \n> s/git the one/git is not/\n\nYou mean s/it's not git/git is not/\n\nFine by me.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"425106","messageId":"875yzda2q9.fsf@vuxu.org","threadId":"55742","inReplyTo":"20210520040725.133848-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Leah Neukirchen","fromEmail":"leah@vuxu.org","sentAt":"2021-05-20T14:39:42Z","receivedAt":"2021-05-20T14:41:14Z","isPatch":true,"sender":{"key":"leah@vuxu.org","avatar":null},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\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 LESS variable set\n>  3. Have color.ui enabled\n>  4. Not have color.pager disabled\n>  5. Not have color.man disabled\n>  6. Not have NO_COLOR set\n>  7. Not have git with stdout directed to a file\n\nI can't review the code thoroughly right now, but if it works as\ndescribed here, +1 from me.\n\n-- \nLeah Neukirchen  <leah@vuxu.org>  https://leahneukirchen.org\n"},{"id":"425113","messageId":"b58cbdcc-abd3-ec82-7d8d-771f47c484ff@gmail.com","threadId":"55742","inReplyTo":"60a66b11d6ffd_2448320885@natae.notmuch","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-05-20T15:13:47Z","receivedAt":"2021-05-20T15:13:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/05/2021 14:58, Felipe Contreras wrote:\n> Phillip Wood wrote:\n>> On 20/05/2021 05:07, 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>> I think there is a distinction between 'diff' and 'grep' where we are\n>> generating the content and help where we are running man\n> \n> It makes a difference for git developers, not for the user.\n> \n> The user doesn't care how the output of `git grep` was generated, all\n> she sees is that it's different from `grep`. It's in fact more\n> surprising than a difference in `git help` because it's even the same\n> comand.\n> \n> Maybe if the command was `git man` they would be equally surprising, but\n> it's not, in fact, `git help` can be used to 1) output directly to the\n> terminal 2) view in a browser, 3) view in info program, 4) view man page\n> in woman, 5) view the man page in koqueror 6) view the man page in man.\n> \n> Only in one case among many would the user expect to see man, therefore\n> a colorized `git grep` is more surprising.\n\nI'm not sure I follow that argument\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 the LESS variable to render bold, underlined, and standout\n>>> text with colors in the less pager.\n>>>\n>>> Bold is rendered as red, underlined as blue, and standout (prompt and\n>>> highlighted search) as inverse magenta.\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.\n>>\n>> However if they have specified the colors they would like by using the\n>> LESS_TERMCAP_xx environment variables that the previous versions of this\n>> patch used their choice is overridden by this new patch.\n> \n> That is true. We could add a check for that:\n> \n>    if (getenv(\"LESS_TERMCAP_md\"))\n>            return;\n> \n> However, it may not be necessary since many of the tips online set these\n> variables inside a function.\n\nThe only person who has tested this patch has reported a problem with \nit, it seems unlikely that no other users will have similar issues. The \nArch Linux wiki (which I think is probably where I got the idea to set \nLESS_TERMCAP_xx) has a section on less[1] suggesting that \nLESS_TERMCAP_xx is exported unconditionally in .bashrc, and a later on \nman suggesting setting them in a function.\n\nBest Wishes\n\nPhillip\n\n[1]\nhttps://wiki.archlinux.org/title/Color_output_in_console#less\n\n>> I've got LESS_TERMCAP_xx set and running\n>> \tLESS='Dd+r$Du+b$Ds' man git add\n>> changes the output colors\n> \n> You have them set in the environtment? Not inside a function like\n> man () { ... command man \"$@\" } ?\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>>\n>> s/git the one/git is not/\n> \n> You mean s/it's not git/git is not/\n> \n> Fine by me.\n> \n> Cheers.\n> \n"},{"id":"425126","messageId":"60a6877fa8389_2747c20842@natae.notmuch","threadId":"55742","inReplyTo":"b58cbdcc-abd3-ec82-7d8d-771f47c484ff@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-20T15:59:59Z","receivedAt":"2021-05-20T16:00:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 20/05/2021 14:58, Felipe Contreras wrote:\n> > Phillip Wood wrote:\n> >> On 20/05/2021 05:07, 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> >> I think there is a distinction between 'diff' and 'grep' where we are\n> >> generating the content and help where we are running man\n> > \n> > It makes a difference for git developers, not for the user.\n> > \n> > The user doesn't care how the output of `git grep` was generated, all\n> > she sees is that it's different from `grep`. It's in fact more\n> > surprising than a difference in `git help` because it's even the same\n> > comand.\n> > \n> > Maybe if the command was `git man` they would be equally surprising, but\n> > it's not, in fact, `git help` can be used to 1) output directly to the\n> > terminal 2) view in a browser, 3) view in info program, 4) view man page\n> > in woman, 5) view the man page in koqueror 6) view the man page in man.\n> > \n> > Only in one case among many would the user expect to see man, therefore\n> > a colorized `git grep` is more surprising.\n> \n> I'm not sure I follow that argument\n\nDo this:\n\n  git config --global help.format html\n  git help git\n\nDo you see a man page on less?\n \n> >>> If the user has already set the LESS variable in his/her environment,\n> >>> that is respected, and nothing changes.\n> >>\n> >> However if they have specified the colors they would like by using the\n> >> LESS_TERMCAP_xx environment variables that the previous versions of this\n> >> patch used their choice is overridden by this new patch.\n> > \n> > That is true. We could add a check for that:\n> > \n> >    if (getenv(\"LESS_TERMCAP_md\"))\n> >            return;\n> > \n> > However, it may not be necessary since many of the tips online set these\n> > variables inside a function.\n> \n> The only person who has tested this patch has reported a problem with \n> it, it seems unlikely that no other users will have similar issues.\n\nThe check above will fix your problem, will it not?\n\n-- \nFelipe Contreras\n"},{"id":"425137","messageId":"6dc0fcee-3415-e6f9-df30-c97de4385f56@gmail.com","threadId":"55742","inReplyTo":"60a6877fa8389_2747c20842@natae.notmuch","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-05-20T18:00:15Z","receivedAt":"2021-05-20T18:00:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/05/2021 16:59, Felipe Contreras wrote:\n> Phillip Wood wrote:\n>> On 20/05/2021 14:58, Felipe Contreras wrote:\n>>> Phillip Wood wrote:\n>>>> On 20/05/2021 05:07, Felipe Contreras wrote:\n>>>>> [...]\n>>>>> If the user has already set the LESS variable in his/her environment,\n>>>>> that is respected, and nothing changes.\n>>>>\n>>>> However if they have specified the colors they would like by using the\n>>>> LESS_TERMCAP_xx environment variables that the previous versions of this\n>>>> patch used their choice is overridden by this new patch.\n>>>\n>>> That is true. We could add a check for that:\n>>>\n>>>     if (getenv(\"LESS_TERMCAP_md\"))\n>>>             return;\n>>>\n>>> However, it may not be necessary since many of the tips online set these\n>>> variables inside a function.\n>>\n>> The only person who has tested this patch has reported a problem with\n>> it, it seems unlikely that no other users will have similar issues.\n> \n> The check above will fix your problem, will it not?\n\nYes it will if it is implemented which was not clear as your message \nsuggested it may not be necessary. I think it would be safer to check \nLESS_TERMCAP_{md,us,so} and not set LESS if any of them are set as it is \npossible a user may only override some of them.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"425181","messageId":"xmqqbl94smjb.fsf@gitster.g","threadId":"55742","inReplyTo":"842221d6-51c4-e08a-4299-c4efb8bf1dcb@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-21T05:06:48Z","receivedAt":"2021-05-21T05:06:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 20/05/2021 05:07, 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> I think there is a distinction between 'diff' and 'grep' where we are\n> generating the content and help where we are running man - I would \n> expect a man page to look the same whether it is displayed by 'man git\n> foo' or 'git help foo'\n\n... as long as the user chooses \"man\" backend, that is.  And I tend\nto agree, but that is our expectation.\n\nIf we added this new mode of driving the same \"man\" but with\ndifferent environment variables exported to tweak how \"less\"\nbehaves, and taught it to builtin/help.c::exec_viewer() and\nbuiltin/help.c::man_viewer_list, that might become more palatable in\nthe sense that we can view it as feeding the same manual page to\nthis another \"man\" that behaves differently from the plain \"man\",\njust like we can feed it to \"woman\" or \"konqueror\" to get a different\nview.  So those (like you and I) who expect a man page to look the\nsame in \"man git foo\" and \"git help -m foo\" can keep using our current\nconfiguration, while those who want yet another variant of \"man\" output\nin addition to the current \"man\", \"woman\", and \"konqueror\" can choose\nit and get \"colorized\" output.\n\nBy the way, this new round mentions NO_COLOR, and while I think it\nis good idea to teach git to honor it, I think it does it at a wrong\nlevel.  Each ui driver that is optionally capable of coloring its\noutput shouldn't have to care, and the right level is either inside\nwant_color() or its helper function check_auto_color(), both in\ncolor.c, to say \"the user hasn't configured the output of this\nsubcommand for coloring, and by default we use color under certain\nconditions (i.e. \"auto\"), but we decide not to use color because\nNO_COLOR environment is set before even checking these \"auto\"\nconditions.\n"},{"id":"425185","messageId":"YKdy5jhHgG2who27@coredump.intra.peff.net","threadId":"55742","inReplyTo":"xmqqbl94smjb.fsf@gitster.g","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-21T08:44:22Z","receivedAt":"2021-05-21T08:44:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 21, 2021 at 02:06:48PM +0900, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n> > On 20/05/2021 05:07, 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> > I think there is a distinction between 'diff' and 'grep' where we are\n> > generating the content and help where we are running man - I would \n> > expect a man page to look the same whether it is displayed by 'man git\n> > foo' or 'git help foo'\n> \n> ... as long as the user chooses \"man\" backend, that is.  And I tend\n> to agree, but that is our expectation.\n> \n> If we added this new mode of driving the same \"man\" but with\n> different environment variables exported to tweak how \"less\"\n> behaves, and taught it to builtin/help.c::exec_viewer() and\n> builtin/help.c::man_viewer_list, that might become more palatable in\n> the sense that we can view it as feeding the same manual page to\n> this another \"man\" that behaves differently from the plain \"man\",\n> just like we can feed it to \"woman\" or \"konqueror\" to get a different\n> view.  So those (like you and I) who expect a man page to look the\n> same in \"man git foo\" and \"git help -m foo\" can keep using our current\n> configuration, while those who want yet another variant of \"man\" output\n> in addition to the current \"man\", \"woman\", and \"konqueror\" can choose\n> it and get \"colorized\" output.\n\nI still don't understand what we gain by making this a Git feature, as\nall of the changed behavior is totally within the program we are\ncalling. Imagine that konqueror (or an html viewer like firefox) had an\noption to set its color scheme from the command line. Should we\nintroduce a new baked-in fancy-konqueror backend that is \"run the tool\nwith a tweaked color scheme\"?\n\nWhy would we do that versus saying: if you want to change the colors in\nthe tool that Git calls, then configure the tool?\n\nIf you like to see colors in manpages, why not configure \"man\" (either\nby setting these environment variables all the time, or by triggering\nthem in MANPAGER)? And then Git doesn't have to care either way; it is\ncalling \"man\" which does what the user wants, colors or no. If you\nreally for some reason only want colorized man pages when called via\n\"git help\", then why not set man.fancy.cmd to invoke your preferred\nconfig?\n\nIf those configurations are awkward to trigger via man (e.g., putting\nescapes into termcap variables), isn't that something that could be\nimproved in man? And then it would benefit everyone who uses man, not\njust Git.\n\n-Peff\n"},{"id":"425223","messageId":"60a7f14a4f0ee_5503920867@natae.notmuch","threadId":"55742","inReplyTo":"6dc0fcee-3415-e6f9-df30-c97de4385f56@gmail.com","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-21T17:43:38Z","receivedAt":"2021-05-21T17:43:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 20/05/2021 16:59, Felipe Contreras wrote:\n> > Phillip Wood wrote:\n> >> On 20/05/2021 14:58, Felipe Contreras wrote:\n\n> >>> That is true. We could add a check for that:\n> >>>\n> >>>     if (getenv(\"LESS_TERMCAP_md\"))\n> >>>             return;\n> >>>\n> >>> However, it may not be necessary since many of the tips online set these\n> >>> variables inside a function.\n> >>\n> >> The only person who has tested this patch has reported a problem with\n> >> it, it seems unlikely that no other users will have similar issues.\n> > \n> > The check above will fix your problem, will it not?\n> \n> Yes it will if it is implemented which was not clear as your message \n> suggested it may not be necessary.\n\nIt was a maybe.\n\n> I think it would be safer to check LESS_TERMCAP_{md,us,so} and not set\n> LESS if any of them are set as it is possible a user may only override\n> some of them.\n\nSure, if we could set 6 variables before, we can check for 3 afterwards.\n\n-- \nFelipe Contreras\n"},{"id":"425224","messageId":"60a7f3d6c55e3_5503920844@natae.notmuch","threadId":"55742","inReplyTo":"xmqqbl94smjb.fsf@gitster.g","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-21T17:54:30Z","receivedAt":"2021-05-21T17:54:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n> > On 20/05/2021 05:07, 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> > I think there is a distinction between 'diff' and 'grep' where we are\n> > generating the content and help where we are running man - I would \n> > expect a man page to look the same whether it is displayed by 'man git\n> > foo' or 'git help foo'\n> \n> ... as long as the user chooses \"man\" backend, that is.  And I tend\n> to agree, but that is our expectation.\n> \n> If we added this new mode of driving the same \"man\" but with\n> different environment variables exported to tweak how \"less\"\n> behaves, and taught it to builtin/help.c::exec_viewer() and\n> builtin/help.c::man_viewer_list, that might become more palatable in\n> the sense that we can view it as feeding the same manual page to\n> this another \"man\" that behaves differently from the plain \"man\",\n> just like we can feed it to \"woman\" or \"konqueror\" to get a different\n> view.  So those (like you and I) who expect a man page to look the\n> same in \"man git foo\" and \"git help -m foo\" can keep using our current\n> configuration, while those who want yet another variant of \"man\" output\n> in addition to the current \"man\", \"woman\", and \"konqueror\" can choose\n> it and get \"colorized\" output.\n\nSo... \"mancolor\"?\n\n> By the way, this new round mentions NO_COLOR, and while I think it\n> is good idea to teach git to honor it, I think it does it at a wrong\n> level.\n\nOther people have already mentioend the FAQ [1]:\n\n  It is reasonable to configure certain software such as a text editor\n  to use color or other ANSI attributes sparingly (such as the reverse\n  attribute for a status bar) while still desiring that other software\n  not add color unless configured to.\n\nAt whatever level it's chosen it shouldn't blatantly disable all color.\n\n> Each ui driver that is optionally capable of coloring its\n> output shouldn't have to care,\n\nBut they do have to care. The purpose of NO_COLOR is not to disable all\ncolor, but to disable annoying color.\n\nhttps://no-color.org/\n\n-- \nFelipe Contreras\n"},{"id":"425225","messageId":"60a7f57fe3301_5503920831@natae.notmuch","threadId":"55742","inReplyTo":"YKdy5jhHgG2who27@coredump.intra.peff.net","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-21T18:01:35Z","receivedAt":"2021-05-21T18:01:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Fri, May 21, 2021 at 02:06:48PM +0900, Junio C Hamano wrote:\n> \n> > Phillip Wood <phillip.wood123@gmail.com> writes:\n> > \n> > > On 20/05/2021 05:07, 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> > > I think there is a distinction between 'diff' and 'grep' where we are\n> > > generating the content and help where we are running man - I would \n> > > expect a man page to look the same whether it is displayed by 'man git\n> > > foo' or 'git help foo'\n> > \n> > ... as long as the user chooses \"man\" backend, that is.  And I tend\n> > to agree, but that is our expectation.\n> > \n> > If we added this new mode of driving the same \"man\" but with\n> > different environment variables exported to tweak how \"less\"\n> > behaves, and taught it to builtin/help.c::exec_viewer() and\n> > builtin/help.c::man_viewer_list, that might become more palatable in\n> > the sense that we can view it as feeding the same manual page to\n> > this another \"man\" that behaves differently from the plain \"man\",\n> > just like we can feed it to \"woman\" or \"konqueror\" to get a different\n> > view.  So those (like you and I) who expect a man page to look the\n> > same in \"man git foo\" and \"git help -m foo\" can keep using our current\n> > configuration, while those who want yet another variant of \"man\" output\n> > in addition to the current \"man\", \"woman\", and \"konqueror\" can choose\n> > it and get \"colorized\" output.\n> \n> I still don't understand what we gain by making this a Git feature,\n\nWhat do we gain by making `git diff` output color?\n\n> Why would we do that versus saying: if you want to change the colors in\n> the tool that Git calls, then configure the tool?\n\nOnce again... How?\n\n> If you like to see colors in manpages, why not configure \"man\" (either\n> by setting these environment variables all the time, or by triggering\n> them in MANPAGER)?\n\nLet me try that...\n\n  MANPAGER=\"less -Dd+r -Du+b -Ds+m\" git help git\n\nIt doesn't work.\n\n> If those configurations are awkward to trigger via man (e.g., putting\n> escapes into termcap variables), isn't that something that could be\n> improved in man? And then it would benefit everyone who uses man, not\n> just Git.\n\nSure. In the meantime let's make `git help` output with color just like\n`git diff`.\n\nCheers.\n\n(and good luck convincing a GNU project of anything)\n\n-- \nFelipe Contreras\n"},{"id":"425238","messageId":"YKgXXCvWYI9rjKJT@coredump.intra.peff.net","threadId":"55742","inReplyTo":"60a7f57fe3301_5503920831@natae.notmuch","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-21T20:26:04Z","receivedAt":"2021-05-21T20:26:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 21, 2021 at 01:01:35PM -0500, Felipe Contreras wrote:\n\n> > I still don't understand what we gain by making this a Git feature,\n> \n> What do we gain by making `git diff` output color?\n\nHuh? Git is outputting the diff. Who else would output the color?\n\n> > Why would we do that versus saying: if you want to change the colors in\n> > the tool that Git calls, then configure the tool?\n> \n> Once again... How?\n\nBy exporting the environment variables that ask it to do so, just like\nyou showed already?\n\n> > If you like to see colors in manpages, why not configure \"man\" (either\n> > by setting these environment variables all the time, or by triggering\n> > them in MANPAGER)?\n> \n> Let me try that...\n> \n>   MANPAGER=\"less -Dd+r -Du+b -Ds+m\" git help git\n> \n> It doesn't work.\n\n  ESC=$(printf '\\33')\n  export MANCOLORS=\"LESS_TERMCAP_md=$ESC[31m LESS_TERMCAP_me=$ESC[0m\"\n  export MANPAGER='sh -c \"eval $MANCOLORS less\"'\n  man ls\n  git help git\n\nAt least on Linux, $MANPAGER is some weird limbo that is not run with\nthe shell, but not just a simple command. Hence the extra layer of \"sh\".\n\nIf I were actually planning to use this myself, I'd probably put it in a\n\"manpager\" script in my $PATH and just do MANPAGER=manpager.\n\n-Peff\n"},{"id":"425241","messageId":"60a828cebd2f1_77e4f208b2@natae.notmuch","threadId":"55742","inReplyTo":"YKgXXCvWYI9rjKJT@coredump.intra.peff.net","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-21T21:40:30Z","receivedAt":"2021-05-21T21:40:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Fri, May 21, 2021 at 01:01:35PM -0500, Felipe Contreras wrote:\n> \n> > > I still don't understand what we gain by making this a Git feature,\n> > \n> > What do we gain by making `git diff` output color?\n> \n> Huh? Git is outputting the diff. Who else would output the color?\n\nDo you think our users know or care which binary has the final\nconnection to the tty?\n\nMany probably think git is sending the output to `diff --color -u`, and\nit doesn't matter at all.\n\n> > > Why would we do that versus saying: if you want to change the colors in\n> > > the tool that Git calls, then configure the tool?\n> > \n> > Once again... How?\n> \n> By exporting the environment variables that ask it to do so, just like\n> you showed already?\n\nExporting MANPAGER is not enough. That would only work on systems that\nhave SGR disabled.\n\nThe user would have to in addition export GROFF_NO_SGR=1, but that would\ndisble groff color for everything, which may not be what the user wants.\n\nThere is no MANGROFFNOSGR.\n\n> > > If you like to see colors in manpages, why not configure \"man\" (either\n> > > by setting these environment variables all the time, or by triggering\n> > > them in MANPAGER)?\n> > \n> > Let me try that...\n> > \n> >   MANPAGER=\"less -Dd+r -Du+b -Ds+m\" git help git\n> > \n> > It doesn't work.\n> \n>   ESC=$(printf '\\33')\n>   export MANCOLORS=\"LESS_TERMCAP_md=$ESC[31m LESS_TERMCAP_me=$ESC[0m\"\n>   export MANPAGER='sh -c \"eval $MANCOLORS less\"'\n>   man ls\n>   git help git\n\nThat still doesn't work here.\n\nhttps://snipboard.io/GmhRtU.jpg\n\nI see the default docbook colos generated by groff, but not the ones you\nspecified (both on `man` and `git help`).\n\nI need to do this as well:\n\n  export GROFF_NO_SGR=1\n\nYour system probably has groff's SGR disabled in /usr/share/groff/site-tmac/man.local\n\nIt's not that simple.\n\nThere is in fact a way to configure man to do what we want here but if\n*nobody* knows what that way is, then does it really matter?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"425295","messageId":"YKjU+/mGzWoqe88V@coredump.intra.peff.net","threadId":"55742","inReplyTo":"60a828cebd2f1_77e4f208b2@natae.notmuch","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-22T09:55:07Z","receivedAt":"2021-05-22T09:55:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 21, 2021 at 04:40:30PM -0500, Felipe Contreras wrote:\n\n> Jeff King wrote:\n> > On Fri, May 21, 2021 at 01:01:35PM -0500, Felipe Contreras wrote:\n> > \n> > > > I still don't understand what we gain by making this a Git feature,\n> > > \n> > > What do we gain by making `git diff` output color?\n> > \n> > Huh? Git is outputting the diff. Who else would output the color?\n> \n> Do you think our users know or care which binary has the final\n> connection to the tty?\n\nYes. If we are telling them that \"git help git\" is using \"man\", which we\ndo, then I think they should expect it to behave like \"man\".\n\nMoreover, I think that if they like colorized manpages, they'd probably\nwant them when running \"man\" themselves.\n\n-Peff\n"},{"id":"425302","messageId":"362a8b5b-84cf-079d-a4c7-c714ed3a2f07@iee.email","threadId":"55742","inReplyTo":"YKjU+/mGzWoqe88V@coredump.intra.peff.net","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-05-22T12:43:47Z","receivedAt":"2021-05-22T12:43:52Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 22/05/2021 10:55, Jeff King wrote:\n> On Fri, May 21, 2021 at 04:40:30PM -0500, Felipe Contreras wrote:\n>\n>> Jeff King wrote:\n>>> On Fri, May 21, 2021 at 01:01:35PM -0500, Felipe Contreras wrote:\n>>>\n>>>>> I still don't understand what we gain by making this a Git feature,\n>>>> What do we gain by making `git diff` output color?\n>>> Huh? Git is outputting the diff. Who else would output the color?\n>> Do you think our users know or care which binary has the final\n>> connection to the tty?\n> Yes. If we are telling them that \"git help git\" is using \"man\", which we\n> do, then I think they should expect it to behave like \"man\".\n>\n> Moreover, I think that if they like colorized manpages, they'd probably\n> want them when running \"man\" themselves.\n>\n> -Peff\nAnd we have the whole Git for Windows community who don't have `man`\nanyway...\nIt's a bit of a conundrum, especially when considering all the\n'terminals' Windows folk maybe using.\n"},{"id":"425344","messageId":"60a96e76a4b20_857e92085c@natae.notmuch","threadId":"55742","inReplyTo":"YKjU+/mGzWoqe88V@coredump.intra.peff.net","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-22T20:49:58Z","receivedAt":"2021-05-22T20:51:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> On Fri, May 21, 2021 at 04:40:30PM -0500, Felipe Contreras wrote:\n> \n> > Jeff King wrote:\n> > > On Fri, May 21, 2021 at 01:01:35PM -0500, Felipe Contreras wrote:\n> > > \n> > > > > I still don't understand what we gain by making this a Git feature,\n> > > > \n> > > > What do we gain by making `git diff` output color?\n> > > \n> > > Huh? Git is outputting the diff. Who else would output the color?\n> > \n> > Do you think our users know or care which binary has the final\n> > connection to the tty?\n> \n> Yes. If we are telling them that \"git help git\" is using \"man\", which we\n> do, then I think they should expect it to behave like \"man\".\n\nBut we are not telling them.\n\nSoftware is not in the business of explaining users exactly what it is\ndoing. Software is in the business of being useful to users, and in\norder to do that it must remain as silent as possible while achieving\nwhat the user potentially wants.\n\nUnless we throw an advice(\"this command runs man\"), then we are not\ntelling them.\n\nIf a dilligent user does `git help help` they might learn about this\nfact, but we didn't tell them, they found out.\n\n> Moreover, I think that if they like colorized manpages, they'd probably\n> want them when running \"man\" themselves.\n\nThis doesn't matter.\n\nThe user might have \"configured\" man like this:\n\n  man() {\n      LESS_TERMCAP_md=$'\\e[01;31m' \\\n      LESS_TERMCAP_me=$'\\e[0m' \\\n      LESS_TERMCAP_so=$'\\e[01;44;33m' \\\n      LESS_TERMCAP_se=$'\\e[0m' \\\n      LESS_TERMCAP_us=$'\\e[01;32m' \\\n      LESS_TERMCAP_ue=$'\\e[0m' \\\n      command man \"$@\"\n  }\n\nGit isn't going utilize that.\n\nArch Linux recommends the above, and so does many online resources.\n\nSo even if it's the case what you said, that they want colorized man\npages, *and* they have man configured, that doesn't matter.\n\nIn addition, not everyone is a Linux guru. Some might want colorized man\npages, but not know how to get them.\n\nI myself only learned it was possible to configure that about a year ago\nwhen reading Arch Linux's installation guide. Luckily I clicked \"Color\noutput in console\", even though I thought I already had most console\nsoftware configured.\n\nI have 20 years of experience using Linux. Some people have less.\n\nYou presume too much of our users.\n\nAnd you still haven't explained how they can properly configure\ncolorized man pages for both man and git, in a way that works in all\ndistributions.\n\n[1] https://wiki.archlinux.org/title/Color_output_in_console\n\n-- \nFelipe Contreras\n"},{"id":"425345","messageId":"60a96f581a80a_857e920890@natae.notmuch","threadId":"55742","inReplyTo":"362a8b5b-84cf-079d-a4c7-c714ed3a2f07@iee.email","subject":"Re: [PATCH v4] help: colorize man pages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-22T20:53:44Z","receivedAt":"2021-05-22T20:53:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Philip Oakley wrote:\n> And we have the whole Git for Windows community who don't have `man`\n> anyway...\n\nIf they don't have man, then they won't be affected in any way.\n\n-- \nFelipe Contreras\n"}]}