[PATCH v2] pager: migrate heavily-used extension into core

[Augie Fackler]

> On Feb 6, 2017, at 12:40, Jun Wu <quark at fb.com> wrote:
> 
> Excerpts from Augie Fackler's message of 2017-02-05 22:24:39 -0500:
>>> On Sun, Feb 5, 2017 at 1:44 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>>> I like the direction of this patch, but this still involves a behavior
>>> change. If PAGER variable is set but pager extension disabled, pager will
>>> be started wrongly.
>>> 
>>> I'm inclined to *not* add special code to see if the old pager extension
>>> has been disabled. That's achievable by disabling the pager instead.
>>> This would require a release note, of course (just in case anyone reads
>>> them).
>> I’m conflicted here: I’ve been chasing my tail on a problem with emacs
>> tramp-mode for months, and finally root-caused it to a missing flag in my
>> pager settings for less, only triggered by emacs running hg over ssh. It
>> was pretty baffling.
> 
> I guess the TTY check could prevent pager from running incorrectly?

That's my thought too, but pager already looks for a tty today. I think tramp actually requests a tty on the other end of the connection for some reason. I haven't had a chance to delve deeper.

> On the other hand, it’s clearly the most-right choice to have the pager on
>> as part of the recommended configuration. I’ve been using it (as an
>> experiment) at work for over a year, and I’ve finally gotten used to it
>> and (for the most part) like it.
>> 
>> What do other people think?
> 
> I think the point of moving pager to core is to make it more accessible.
> If a new user only needs to set PAGER without touching .hgrc to use a pager,
> that sounds like a step forward to me.

Yes, we should definitely make pager easier to use. My own informal surveys of users are that even a setting in hgrc would be better than an extension, because there's a perception that an extension is somehow dangerous.

> If BC is a concern, some temporary deprecation warnings might be helpful.
> 
> The patch seems to conflict with Simon's stdout change - a rebase is
> probably needed. Otherwise it looks good to me, I have especially checked
> chg compatibility.

Cool. I'll make a pass on this patch's substance today and try and offer any comments to save time on a v3.

Thanks!
Augie