All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org,
	Felipe Contreras <felipe.contreras@gmail.com>,
	Jeff King <peff@peff.net>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v7] help: colorize man pages if man.color=true under less(1)
Date: Mon, 21 Jun 2021 21:08:20 +0200	[thread overview]
Message-ID: <87mtrj2faq.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqfsxbika3.fsf@gitster.g>


On Mon, Jun 21 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> So, in order for this change to have any effect:
>>
>>  1. color.man=true must be set in the config
>>  2. The user must use less
>>  3. Not have the same LESS_TERMCAP variables set (we call setenv(3) with overwrite=0)
>>  4. Have color.ui enabled
>>  5. Not have color.pager disabled
>>  6. Not have git with stdout directed to a file
>>
>> 1. https://lore.kernel.org/git/87tun1qp91.fsf@evledraar.gmail.com/
>> 2. https://unix.stackexchange.com/questions/119/colors-in-man-pages/147
>>
>> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>
>> On Tue, Jun 08 2021, Junio C Hamano wrote:
>>
>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>>
>>>> I've been running with this on my personal git build since May 26th. I
>>>> haven't had any issues with it, and I like the new coloring.
>>>> ...
>>>> I think this is a good example of a change that we're better off just
>>>> merging down and then reverting if the wider audience of git users hates
>>>> it, rather than trying to come to some perfect consensus here
>>>> on-list.
>>>
>>> My impression was tht we already had a rough consensus here on-list
>>> that it may be good to educate users who like this "new coloring"
>>> like you do to configure their "less", so that they consistently get
>>> the "new coloring" they like whether they are doing "git help git",
>>> "man git", or even "man ls", and the approach the posted patch takes
>>> will not help (it only affects "git help git" among these).
>>>
>>> I'd rather not to take it.
>>
>> Fair enough, here's a version I think you and others will find
>> acceptable then. It allows users like me who like this to explicitly
>> opt-in via color.man=true.
>
> Not really.
>
> [snip] I think it would be easier to understand to end-users
> if this were exposed as a new "mode", like "git help --web" and "git
> help --info" are different modes from the "git help --man",
> something like "git help --fancy-man" (or whatever is easy to type
> and explain, and also add it to the variants help.format knows about
> to make it easy to set the default).
>
> One advantage of doing so is that we do not have to worry about "ah,
> user has LESS_BLAH environment variable so we should disable this
> new mode here" etc.  As long as the new mode is requested either via
> the command line option or help.format configuration, it can
> completely take it over.  That simplifies the necessary explanation
> given to the users quite a lot, no?

The interaction between "git help" and "man"/"less" doesn't really have
an equivalent in the rest of git as far as color output goes. Usually we
emit colors via our own programs.

But no, I think it makes the most sense to consider this orthagonal to
help.format=man or man.viewer=<cmd>.

We're not invoking a different man viewer or command, we're just
expecting that mode to invoke the pager, and if that pager is less to
have these variables tweak our color preferences.

> [unsnip] Since the implementation of the posted patch, as I understand it,
> does not aim to affect both "git help -m foo" and "man git-foo"
> identically, 

Aside from this patch I don't think it makes sense to view git's UI and
interaction with the pager like that.

To probably >95% of our users the "we invoke the pager" is just a
technical implementation details they're not aware of. Git just has
pretty colors by default, so I don't think it's jarring that "git help
xyz" and "man git-xyz" look different.

We also don't try to maintain the UI that:

    git cmd >file &&
    pager file

Gives you the same UX as:

    # Invokes pager(1) for you
    git cmd

Since we set e.g. PAGER_ENV already, I believe this was brought up in
past discussions. So we're already past the point of git adding its own
magic custom options to feed to the pager.

So I don't see the problem with having "a bit more like PAGER_ENV"
hidden behind a color.man=true config option in this case.

>> 	----
>> 	git-config - Get and set [...]
>>
>> 	SYNOPSIS
>> 	--------
>> 	[...]
>> 	'git config' [<file-option>] [...]
>> 	[...]
>> 	The `--type=<type>` option instructs 'git config' to ensure [...]
>>
>> Will have "NAME" and "SECTION" shown as BOLD RED instead of BOLD, "git
>> config" and other '-quoted parts in BLUE UNDERLINE instead of
>> UNDERLINE, and `--type=<type>` and other `-quoted parts in RED BOLD
>> instead of BOLD. The "Standout" setting is then used for the user's
>> own search bar (invoked with "/") and prompt. See [2] for more
>> examples
>
> There are BOLD RED and READ BOLD; are they differently rendered?

Yes, that's just an omission/mistake. It should be the "RED BOLD",
i.e. "<COLOR> <ATTR>".

  parent reply	other threads:[~2021-06-21 19:23 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
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 [this message]
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=87mtrj2faq.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.