Patchwork [10,of,10,V5] pager: do not read from environment variable

login
register
mail settings
Submitter Jun Wu
Date March 27, 2017, 6:02 a.m.
Message ID <5641b83eff817bdb0d7a.1490594529@localhost.localdomain>
Download mbox | patch
Permalink /patch/19750/
State Accepted
Headers show

Comments

Jun Wu - March 27, 2017, 6:02 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1490589827 25200
#      Sun Mar 26 21:43:47 2017 -0700
# Node ID 5641b83eff817bdb0d7adeb24c7383653ef15617
# Parent  a354f13a30b93943635224fcd90f43019241e23e
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 5641b83eff81
pager: do not read from environment variable

$PAGER is converted to the pager.pager config item. So it's no longer
necessary to read $PAGER in ui.pager().
Ryan McElroy - March 28, 2017, 8:42 a.m.
On 3/27/17 7:02 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1490589827 25200
> #      Sun Mar 26 21:43:47 2017 -0700
> # Node ID 5641b83eff817bdb0d7adeb24c7383653ef15617
> # Parent  a354f13a30b93943635224fcd90f43019241e23e
> pager: do not read from environment variable

This series looks good to me. It's a great behavior improvement and 
cleanup that is well-executed.

I'll mark as pre-reviewed in patchwork. I have a few small nits I'll 
send out shortly but they don't affect the result and could easily be 
follow-ups and shouldn't block queuing in my opinion.

>
> $PAGER is converted to the pager.pager config item. So it's no longer
> necessary to read $PAGER in ui.pager().
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -850,12 +850,6 @@ class ui(object):
>               return
>   
> -        # TODO: add a "system defaults" config section so this default
> -        # of more(1) can be easily replaced with a global
> -        # configuration file. For example, on OS X the sane default is
> -        # less(1), not more(1), and on debian it's
> -        # sensible-pager(1). We should probably also give the system
> -        # default editor command similar treatment.
> -        envpager = encoding.environ.get('PAGER', 'more')
> -        pagercmd = self.config('pager', 'pager', envpager)
> +        fallbackpager = 'more'
> +        pagercmd = self.config('pager', 'pager', fallbackpager)
>           if not pagercmd:
>               return
>
Yuya Nishihara - March 28, 2017, 12:54 p.m.
On Tue, 28 Mar 2017 09:42:54 +0100, Ryan McElroy wrote:
> On 3/27/17 7:02 AM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1490589827 25200
> > #      Sun Mar 26 21:43:47 2017 -0700
> > # Node ID 5641b83eff817bdb0d7adeb24c7383653ef15617
> > # Parent  a354f13a30b93943635224fcd90f43019241e23e
> > pager: do not read from environment variable
> 
> This series looks good to me. It's a great behavior improvement and 
> cleanup that is well-executed.

Yeah, queued these, thanks.

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -850,12 +850,6 @@  class ui(object):
             return
 
-        # TODO: add a "system defaults" config section so this default
-        # of more(1) can be easily replaced with a global
-        # configuration file. For example, on OS X the sane default is
-        # less(1), not more(1), and on debian it's
-        # sensible-pager(1). We should probably also give the system
-        # default editor command similar treatment.
-        envpager = encoding.environ.get('PAGER', 'more')
-        pagercmd = self.config('pager', 'pager', envpager)
+        fallbackpager = 'more'
+        pagercmd = self.config('pager', 'pager', fallbackpager)
         if not pagercmd:
             return