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

Jun Wu quark at fb.com
Mon Feb 6 12:40:15 EST 2017


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?

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

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.


More information about the Mercurial-devel mailing list