Patchwork [STABLE] pager: wrap _runcommand() no matter if stdout is redirected

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 19, 2017, 2:39 p.m.
Message ID <f3ca8b7e0e2df7507661.1484836764@mimosa>
Download mbox | patch
Permalink /patch/18250/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 19, 2017, 2:39 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1484834492 -32400
#      Thu Jan 19 23:01:32 2017 +0900
# Branch stable
# Node ID f3ca8b7e0e2df7507661adf5957c51e39bc6b5b1
# Parent  262c2be8ea5a4f36fb7348cb5a940cf78f2dffa9
pager: wrap _runcommand() no matter if stdout is redirected

We've made chg utilize the common code path implemented in pager.py (by
815e1cefd082 and 493935e0327a), but the chg server does not always start
with a tty. Because of this, uisetup() of the pager extension could be
skipped on the chg server.

Kudos given to Sean Farley for dogfooding new chg and spotting this problem.
Sean Farley - Jan. 19, 2017, 6:29 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1484834492 -32400
> #      Thu Jan 19 23:01:32 2017 +0900
> # Branch stable
> # Node ID f3ca8b7e0e2df7507661adf5957c51e39bc6b5b1
> # Parent  262c2be8ea5a4f36fb7348cb5a940cf78f2dffa9
> pager: wrap _runcommand() no matter if stdout is redirected
>
> We've made chg utilize the common code path implemented in pager.py (by
> 815e1cefd082 and 493935e0327a), but the chg server does not always start
> with a tty. Because of this, uisetup() of the pager extension could be
> skipped on the chg server.
>
> Kudos given to Sean Farley for dogfooding new chg and spotting this problem.

\o/ This fixes the issue for me!!
Augie Fackler - Jan. 19, 2017, 9:55 p.m.
On Thu, Jan 19, 2017 at 11:39:24PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1484834492 -32400
> #      Thu Jan 19 23:01:32 2017 +0900
> # Branch stable
> # Node ID f3ca8b7e0e2df7507661adf5957c51e39bc6b5b1
> # Parent  262c2be8ea5a4f36fb7348cb5a940cf78f2dffa9
> pager: wrap _runcommand() no matter if stdout is redirected

Queued for stable. Thanks!

>
> We've made chg utilize the common code path implemented in pager.py (by
> 815e1cefd082 and 493935e0327a), but the chg server does not always start
> with a tty. Because of this, uisetup() of the pager extension could be
> skipped on the chg server.
>
> Kudos given to Sean Farley for dogfooding new chg and spotting this problem.
>
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -115,9 +115,6 @@ def _runpager(ui, p):
>          pager.wait()
>
>  def uisetup(ui):
> -    if '--debugger' in sys.argv or not ui.formatted():
> -        return
> -
>      class pagerui(ui.__class__):
>          def _runpager(self, pagercmd):
>              _runpager(self, pagercmd)
> @@ -130,7 +127,7 @@ def uisetup(ui):
>          always = util.parsebool(options['pager'])
>          auto = options['pager'] == 'auto'
>
> -        if not p:
> +        if not p or '--debugger' in sys.argv or not ui.formatted():
>              pass
>          elif always:
>              usepager = True
> diff --git a/tests/test-chg.t b/tests/test-chg.t
> --- a/tests/test-chg.t
> +++ b/tests/test-chg.t
> @@ -32,6 +32,38 @@ long socket path
>
>    $ cd ..
>
> +pager
> +-----
> +
> +  $ cat >> fakepager.py <<EOF
> +  > import sys
> +  > for line in sys.stdin:
> +  >     sys.stdout.write('paged! %r\n' % line)
> +  > EOF
> +
> +enable pager extension globally, but spawns the master server with no tty:
> +
> +  $ chg init pager
> +  $ cd pager
> +  $ cat >> $HGRCPATH <<EOF
> +  > [extensions]
> +  > pager =
> +  > [pager]
> +  > pager = python $TESTTMP/fakepager.py
> +  > EOF
> +  $ chg version > /dev/null
> +  $ touch foo
> +  $ chg ci -qAm foo
> +
> +pager should be enabled if the attached client has a tty:
> +
> +  $ chg log -l1 -q --config ui.formatted=True
> +  paged! '0:1f7b0de80e11\n'
> +  $ chg log -l1 -q --config ui.formatted=False
> +  0:1f7b0de80e11
> +
> +  $ cd ..
> +
>  server lifecycle
>  ----------------
>
> diff --git a/tests/test-pager.t b/tests/test-pager.t
> --- a/tests/test-pager.t
> +++ b/tests/test-pager.t
> @@ -152,6 +152,11 @@ doesn't result in history being paged.
>    summary:     modify a 9
>
>
> +Pager should not start if stdout is not a tty.
> +
> +  $ hg log -l1 -q --config ui.formatted=False
> +  10:46106edeeb38
> +
>  Pager with color enabled allows colors to come through by default,
>  even though stdout is no longer a tty.
>    $ cat >> $HGRCPATH <<EOF
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -115,9 +115,6 @@  def _runpager(ui, p):
         pager.wait()
 
 def uisetup(ui):
-    if '--debugger' in sys.argv or not ui.formatted():
-        return
-
     class pagerui(ui.__class__):
         def _runpager(self, pagercmd):
             _runpager(self, pagercmd)
@@ -130,7 +127,7 @@  def uisetup(ui):
         always = util.parsebool(options['pager'])
         auto = options['pager'] == 'auto'
 
-        if not p:
+        if not p or '--debugger' in sys.argv or not ui.formatted():
             pass
         elif always:
             usepager = True
diff --git a/tests/test-chg.t b/tests/test-chg.t
--- a/tests/test-chg.t
+++ b/tests/test-chg.t
@@ -32,6 +32,38 @@  long socket path
 
   $ cd ..
 
+pager
+-----
+
+  $ cat >> fakepager.py <<EOF
+  > import sys
+  > for line in sys.stdin:
+  >     sys.stdout.write('paged! %r\n' % line)
+  > EOF
+
+enable pager extension globally, but spawns the master server with no tty:
+
+  $ chg init pager
+  $ cd pager
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > pager =
+  > [pager]
+  > pager = python $TESTTMP/fakepager.py
+  > EOF
+  $ chg version > /dev/null
+  $ touch foo
+  $ chg ci -qAm foo
+
+pager should be enabled if the attached client has a tty:
+
+  $ chg log -l1 -q --config ui.formatted=True
+  paged! '0:1f7b0de80e11\n'
+  $ chg log -l1 -q --config ui.formatted=False
+  0:1f7b0de80e11
+
+  $ cd ..
+
 server lifecycle
 ----------------
 
diff --git a/tests/test-pager.t b/tests/test-pager.t
--- a/tests/test-pager.t
+++ b/tests/test-pager.t
@@ -152,6 +152,11 @@  doesn't result in history being paged.
   summary:     modify a 9
   
 
+Pager should not start if stdout is not a tty.
+
+  $ hg log -l1 -q --config ui.formatted=False
+  10:46106edeeb38
+
 Pager with color enabled allows colors to come through by default,
 even though stdout is no longer a tty.
   $ cat >> $HGRCPATH <<EOF