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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
