From: "Kyle J. McKay" <mackyle@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
Yuri <yuri@rawbw.com>
Subject: Re: [PATCH 2/3] setup_pager: set MORE=R
Date: Tue, 4 Feb 2014 18:11:01 -0800 [thread overview]
Message-ID: <62DB6DEF-8B39-4481-BA06-245BF45233E5@gmail.com> (raw)
In-Reply-To: <20140204221220.GA5457@sigill.intra.peff.net>
On Feb 4, 2014, at 14:12, Jeff King wrote:
> On Tue, Jan 21, 2014 at 11:23:30AM -0800, Junio C Hamano wrote:
>
>>> does complicate the point of my series, which was to add more
>>> intimate
>>> logic about how we handle LESS.
>>> ...
>>> return !x || strchr(x, 'R');
>> [...]
>>
>> I am not sure if it is even a good idea for us to have so intimate
>> logic for various pagers in the first place. I'd seriously wonder
>> if it is better to take this position:
>>
>> A platform packager who sets the default pager and/or the
>> default environment for the pager at the build time, or an
>> individual user who tells your Git what pager you want to
>> use and/or with what setting that pager should be run under
>> with environment variables. These people ought to know far
>> better than Git what their specific choices do. Do not try
>> to second-guess them.
>
> Sorry for letting this topic lapse; I wanted to look at some possible
> Makefile improvements, which I'll be posting in a moment. I did want
> to
> address this point, though.
>
> If we are just talking about packagers and defaults (e.g., setting
> MORE=R on FreeBSD), then I agree that the right tool for the job is
> empowering the packagers, and the Makefile knob we've discussed does
> that.
>
[snip]
>
> It seems a shame to me that we cannot do better for such users.
> However, the level of intimacy with the pager is getting to be a bit
> too
> much for my taste, and the saving grace is that not that many people
> set
> LESS themselves (and git is widely enough known that "R" is a common
> recommendation when people do). So it's probably the lesser of two
> evils
> to ignore the situation, and let people who run into it find the
> answers
> on the web.
>
> So I think there is nothing to be done. But I did want to mention
> it in
> case somebody else can come up with some clever solution. :)
I think we can do better without adding intimate pager knowledge. And
at the same time make it a simple matter of tweaking the Makefile to
deal with new pagers on specific systems.
There are two parts to the proposal.
Part 1
Add a new configuration item which I will call "pager.env" for now
that can have multiple values of the form ENV=value (whether multiple
lines or whitespace separated on a single line to be decided later).
On a system where more can support color by setting MORE=-R it might
look like this:
[pager]
env = LESS=-FRSX LV=-c MORE=-R
On a system where more does not it might look like this:
[pager]
env = LESS=-FRSX LV=-c
The default value of pager.env is to be configured in a system-
specific way (i.e. Makefile knob) at build time.
Then, if Git is going to output color AND start a pager (that logic
remains unchanged by this proposal), then it processes the pager.env
value by examining each ENV=value setting and if the environment
variable ENV is NOT already set, then sets it to value before starting
the pager.
This is mostly a clean up and shouldn't really be controversial since
before commit e54c1f2d2 the system basically behaved like this where
pager.env was always set to "LESS=FRSX" and after it behaves as though
pager.env is always set to "LESS=FRSX LV=-c".
Part 2
Alongside the current false/never, always, true/auto settings for
"color.ui" a new "carefully" setting is introduced and the color.ui
default if not set is changed from auto (commit 4c7f1819) to the new
"carefully" setting.
Why a new setting? So that people who have explicitly set color.ui to
auto (or one of the other values) will not be affected by the proposed
new logic.
Another new configuration item which I will call "pager.carefully" for
now is introduced that again can have multiple values of the form
name=ENV. It might look like this:
[pager]
carefully = less=LESS LV=lv more=MORE
Again the default value of pager.carefully can be configured in a
system-specific way (i.e. Makefile knob) at build time.
If color.ui is set to "carefully", then the logic is as follows:
a) Decide whether to output color the same way as for color.ui=auto
b) Select the pager the same way as for color.ui=auto
c) If (a) and (b) have selected a pager AND enabled color output then
check to see if the selected pager appears in pager.carefully as one
of the name values (perhaps a suffix match on the first whitespace-
separated word of the selected pager value).
d) If the selected pager does not appear in pager.carefully, disable
color output.
e) If the selected pager appears in pager.carefully, but the ENV
associated with it does NOT appear in pager.env, disable color output.
f) If the pager appears in pager.carefully, but the ENV associated
with it is already set, disable color output.
g) Otherwise, go ahead with color output as the change in Part 1 above
will properly set the pager's options to enable it.
This has the following advantages:
1. No intimate pager knowledge.
2. Color output will be selected on supported systems by default
assuming the user has not set any pager-specific environment variables.
3. This should prevent the user from ever seeing the ugly ESC[33 and
ESC[m sequences with the default settings.
4. A user who has color.ui=auto set is not affected by this change.
5. Support for additional color pagers is easily added by tweaking the
Makefile.
It has the following disadvantage:
1. A user who has a pager-specific environment variable set for the
default pager will not get color by default even if color would be
supported without first setting color.ui=auto or using an equivalent.
Essentially they will get the pre-1.8.4 behavior.
--Kyle
next prev parent reply other threads:[~2014-02-05 2:11 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-17 0:34 'git log' escape symbols shown as ESC[33 and ESC[m Yuri
2014-01-17 1:47 ` Jeff King
2014-01-17 2:02 ` Yuri
2014-01-17 2:13 ` Jeff King
2014-01-17 2:28 ` Yuri
2014-01-17 2:32 ` Jeff King
2014-01-17 2:46 ` Yuri
2014-01-17 2:29 ` Jonathan Nieder
2014-01-17 2:35 ` Jeff King
2014-01-17 3:21 ` Jeff King
2014-01-17 4:14 ` [PATCH 0/3] improved out-of-the-box color settings Jeff King
2014-01-17 4:21 ` [PATCH 1/3] setup_pager: refactor LESS/LV environment setting Jeff King
2014-01-17 4:21 ` [PATCH 2/3] setup_pager: set MORE=R Jeff King
2014-01-17 7:26 ` Kyle J. McKay
2014-01-17 19:11 ` Junio C Hamano
2014-01-21 5:30 ` Jeff King
2014-01-21 8:42 ` Kyle J. McKay
2014-01-23 2:14 ` Jeff King
2014-01-23 17:22 ` Junio C Hamano
2014-01-17 19:17 ` Junio C Hamano
2014-01-17 19:57 ` Junio C Hamano
2014-01-17 23:42 ` Junio C Hamano
2014-01-21 6:13 ` Jeff King
2014-01-21 19:28 ` Junio C Hamano
2014-01-21 5:49 ` Jeff King
2014-01-21 19:23 ` Junio C Hamano
2014-02-04 22:12 ` Jeff King
2014-02-04 22:17 ` Junio C Hamano
2014-02-04 22:25 ` Jeff King
2014-02-04 22:45 ` Yuri
2014-02-04 22:48 ` Jeff King
2014-02-04 22:54 ` Junio C Hamano
2014-02-04 23:00 ` Yuri
2014-02-05 2:11 ` Kyle J. McKay [this message]
2014-01-17 4:24 ` [PATCH 3/3] pager: disable colors for some known-bad configurations Jeff King
2014-01-17 9:13 ` [PATCH 0/3] improved out-of-the-box color settings Yuri
2014-01-17 20:15 ` 'git log' escape symbols shown as ESC[33 and ESC[m Yuri
2014-02-05 1:24 ` Yuri
2014-02-05 1:33 ` Jeff King
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62DB6DEF-8B39-4481-BA06-245BF45233E5@gmail.com \
--to=mackyle@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
--cc=yuri@rawbw.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).