Re: [PATCH 2/3] setup_pager: set MORE=R

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 21, 2014, 19:23 UTC
Message-ID: <xmqqwqhtuojx.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140121054927.GD5878@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> ...
> 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.

Show 14 quoted lines

> 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.