git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: 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, 21 Jan 2014 11:23:30 -0800	[thread overview]
Message-ID: <xmqqwqhtuojx.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140121054927.GD5878@sigill.intra.peff.net> (Jeff King's message of "Tue, 21 Jan 2014 00:49:27 -0500")

Jeff King <peff@peff.net> writes:

> ...
> does complicate the point of my series, which was to add more intimate
> logic about how we handle LESS.
> ...
>                 return !x || strchr(x, 'R');
>     }
>
>     [...]
>   }
>
> but we are still hard-coding a lot of intelligence about "less" here.

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.

The potential breakage caused by setting MORE=R unconditionally is a
good example.  A careless "intimate logic" may think that any pager
that is called 'more' would behave like traditional 'more', breaking
half the 'more' user population while catering to the other half.

> I
> wonder if the abstraction provided by the Makefile variable is really
> worthwhile. Anybody adding a new line to it would also want to tweak
> pager_can_handle_color to add similar logic.

And that is why I am not enthused by the idea of adding such logic
in the first place.  I view the Makefile customization as a way for
the packager to offer a sensible default for their platform without
touching the code, which is slightly different from your 1. below.

> Taking a step back for a moment, we are getting two things out of such a
> Makefile variable:
>
>   1. An easy way for packager to add intelligence about common pagers on
>      their system.
>
>   2. DRY between git-sh-setup and the C code.
>
> I do like (1), but I do not know if we want to try to encode the "can
> handle color" logic into a Makefile variable. What would it look like?
>
> For (2), an alternate method would be to simply provide "git pager" as C
> code, which spawns the appropriate pager as the C code would. Then
> git-sh-setup can easily build around that.

And as to 2., if the answer to the other issue "do we want our code
to be intimately aware of pager-specific quirks, or do we just want
to give packagers a knob to express their choice of the default?"
resolves to the former, I would think that "git pager" would be not
just a workable alternative, but would be the only viable one.

  reply	other threads:[~2014-01-21 19:23 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 [this message]
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
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=xmqqwqhtuojx.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --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).