Patchwork [2,of,3,V2] pager: skip uisetup if chg is detected

login
register
mail settings
Submitter Jun Wu
Date March 16, 2016, 12:08 p.m.
Message ID <f09ac10a2f99ebc65143.1458130090@x1c>
Download mbox | patch
Permalink /patch/13912/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 16, 2016, 12:08 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457967799 0
#      Mon Mar 14 15:03:19 2016 +0000
# Node ID f09ac10a2f99ebc65143aff19fa73d1ad3024da0
# Parent  bc925c3acec1c4938f49d623e797444d955eb59d
pager: skip uisetup if chg is detected

chg has its own pager implementation that it wants to skip pager's uisetup.
It is currently done by redirecting stdout to /dev/null, which has unintended
side effects. This patch makes pager aware of chg and skip uisetup directly
from pager. We may want to merge chg and pager's pager implementation to
make this unnecessary in the future.
Durham Goode - March 17, 2016, 6:39 a.m.
On 3/16/16 5:08 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457967799 0
> #      Mon Mar 14 15:03:19 2016 +0000
> # Node ID f09ac10a2f99ebc65143aff19fa73d1ad3024da0
> # Parent  bc925c3acec1c4938f49d623e797444d955eb59d
> pager: skip uisetup if chg is detected
>
> chg has its own pager implementation that it wants to skip pager's uisetup.
> It is currently done by redirecting stdout to /dev/null, which has unintended
> side effects.
I'd probably mention what the side effects are in the commit message, so 
if future users see issues with this code they can know what it was 
trying to fix.
Jun Wu - March 17, 2016, 8:08 a.m.
On 03/17/2016 06:39 AM, Durham Goode wrote:
> I'd probably mention what the side effects are in the commit message, so if
> future users see issues with this code they can know what it was trying to
> fix.

The side effects was mentioned in the next patch "chg: do not redirect stdout
to /dev/null". It's ui.write stops working and some error message won't get
printed out. I thought it would be repetitive if I write it twice. Next time I
will put the context in both patches.
Yuya Nishihara - March 18, 2016, 10:35 p.m.
On Wed, 16 Mar 2016 12:08:10 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457967799 0
> #      Mon Mar 14 15:03:19 2016 +0000
> # Node ID f09ac10a2f99ebc65143aff19fa73d1ad3024da0
> # Parent  bc925c3acec1c4938f49d623e797444d955eb59d
> pager: skip uisetup if chg is detected
> 
> chg has its own pager implementation that it wants to skip pager's uisetup.
> It is currently done by redirecting stdout to /dev/null, which has unintended
> side effects. This patch makes pager aware of chg and skip uisetup directly
> from pager. We may want to merge chg and pager's pager implementation to
> make this unnecessary in the future.
> 
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -117,6 +117,11 @@
>      if '--debugger' in sys.argv or not ui.formatted():
>          return
>  
> +    # chg has its own pager implementation
> +    argv = sys.argv[:]
> +    if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], argv):
> +        return

Queued this version, thanks. I don't think this is ideal, but we can refactor
pager handling later.

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -117,6 +117,11 @@ 
     if '--debugger' in sys.argv or not ui.formatted():
         return
 
+    # chg has its own pager implementation
+    argv = sys.argv[:]
+    if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], argv):
+        return
+
     def pagecmd(orig, ui, options, cmd, cmdfunc):
         p = ui.config("pager", "pager", os.environ.get("PAGER"))
         usepager = False