From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Phillip Wood <phillip.wood123@gmail.com>,
Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v6] help: colorize man pages
Date: Tue, 08 Jun 2021 14:35:24 +0200 [thread overview]
Message-ID: <87o8cg34t8.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20210523054454.1188757-1-felipe.contreras@gmail.com>
On Sun, May 23 2021, Felipe Contreras wrote:
> We already colorize tools traditionally not colorized by default, like
> diff and grep. Let's do the same for man.
>
> Our man pages don't contain many useful colors (just blue links),
> moreover, many people have groff SGR disabled, so they don't see any
> colors with man pages.
>
> We can set LESS_TERMCAP variables to render bold and underlined text
> with colors in the pager; a common trick[1].
>
> Bold is rendered as red, underlined as blue, and standout (prompt and
> highlighted search) as inverse cyan.
>
> Obviously this only works when the less pager is used.
>
> If the user already has LESS_TERMCAP variables set in his/her
> environment, those are respected and nothing changes.
>
> A new color configuration is added: `color.man` for the people that want
> to turn this feature off, otherwise `color.ui` is respected.
> Additionally, if color.pager is not enabled, this is disregarded.
>
> Normally check_auto_color() would check the value of `color.pager`, but
> in this particular case it's not git the one executing the pager, but
> man. Therefore we need to check pager_use_color ourselves.
>
> Also--unlike other color.* configurations--color.man=always does not
> make any sense here; `git help` is always run for a tty (it would be very
> strange for a user to do `git help $page > output`, but in fact, that
> works anyway, we don't even need to check if stdout is a tty, but just
> to be consistent we do). So it's simply a boolean in our case.
>
> So, in order for this change to have any effect:
>
> 1. The user must use less
> 2. Not have the same LESS_TERMCAP variables set
> 3. Have color.ui enabled
> 4. Not have color.pager disabled
> 5. Not have color.man disabled
> 7. Not have git with stdout directed to a file
>
> Fortunately the vast majority of our users meet all of the above, and
> anybody who doesn't would not be affected negatively (plus very likely
> comprises a very tiny minority).
>
> [1] https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
>
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Phillip Wood <phillip.wood123@gmail.com>
> Comments-by: Jeff King <peff@peff.net>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
I've been running with this on my personal git build since May 26th. I
haven't had any issues with it, and I like the new coloring.
I for one would like to have this picked up by Junio.
I think this is a good example of a change that we're better off just
merging down and then reverting if the wider audience of git users hates
it, rather than trying to come to some perfect consensus here
on-list.
We have a wider audience running "next" than "seen" (but this didn't
even make "seen"), if this were to make it into a release and users
overwhelmingly dislike it it's no big deal. There's a config option to
turn it off, and/or we could make it opt-in later.
Alternatively this could be opt-in and not fall under the color.ui=auto
umbrella, or only in combination with feature.experimental (or a new
ui.experimental?, which would default to that?).
But in any case if judgement call UI changes are always hidden behind
options we'll never make forward progress on things that are possibly
better defaults (and if they're not, we can always simply revert the
change).
next prev parent reply other threads:[~2021-06-08 12:43 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-23 5:44 [PATCH v6] help: colorize man pages Felipe Contreras
2021-05-24 13:13 ` Phillip Wood
2021-05-24 16:56 ` Felipe Contreras
2021-06-08 12:35 ` Ævar Arnfjörð Bjarmason [this message]
2021-06-08 13:57 ` Junio C Hamano
2021-06-08 17:48 ` Felipe Contreras
2021-06-21 8:34 ` [PATCH v7] help: colorize man pages if man.color=true under less(1) Ævar Arnfjörð Bjarmason
2021-06-21 10:17 ` Phillip Wood
2021-06-21 10:28 ` Junio C Hamano
2021-06-21 18:41 ` Felipe Contreras
2021-06-21 19:08 ` Ævar Arnfjörð Bjarmason
2021-06-23 23:58 ` Jeff King
2021-06-24 1:03 ` Felipe Contreras
2021-06-28 17:37 ` Junio C Hamano
2021-06-28 18:05 ` Felipe Contreras
2021-06-21 15:59 ` Felipe Contreras
2021-06-24 0:08 ` Jeff King
2021-06-29 1:42 ` Junio C Hamano
2021-06-29 1:48 ` Felipe Contreras
2021-06-24 1:44 ` [PATCH v6] help: colorize man pages Felipe Contreras
2021-06-26 2:50 ` [PATCH v8] help: add option to colorize man pages under less Felipe Contreras
2021-06-26 14:49 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o8cg34t8.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=phillip.wood123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).