Patchwork pager: wrap ui._runpager

login
register
mail settings
Submitter Jun Wu
Date Jan. 6, 2017, 5:08 p.m.
Message ID <8e614304b040537dbc73.1483722504@x1c>
Download mbox | patch
Permalink /patch/18128/
State Accepted
Headers show

Comments

Jun Wu - Jan. 6, 2017, 5:08 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1482711944 0
#      Mon Dec 26 00:25:44 2016 +0000
# Node ID 8e614304b040537dbc735acb6c3999a05efd258c
# Parent  011122b3b1c42374fb0489d107418e1be3665ca6
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 8e614304b040
pager: wrap ui._runpager

As discussed at [1], ui._runpager will be the new low-level API accepting a
pager command to actually run the pager. And ui.pager is the high-level API
which reads config directly from self.

This change is necessary for chgserver to override _runpager cleanly.

[1]: www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091656.html
Sean Farley - Jan. 6, 2017, 7:27 p.m.
Jun Wu <quark@fb.com> writes:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1482711944 0
> #      Mon Dec 26 00:25:44 2016 +0000
> # Node ID 8e614304b040537dbc735acb6c3999a05efd258c
> # Parent  011122b3b1c42374fb0489d107418e1be3665ca6
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 8e614304b040
> pager: wrap ui._runpager
>
> As discussed at [1], ui._runpager will be the new low-level API accepting a
> pager command to actually run the pager. And ui.pager is the high-level API
> which reads config directly from self.
>
> This change is necessary for chgserver to override _runpager cleanly.
>
> [1]: www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091656.html

Looks good to me.
Augie Fackler - Jan. 7, 2017, 5:56 p.m.
On Fri, Jan 06, 2017 at 05:08:24PM +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1482711944 0
> #      Mon Dec 26 00:25:44 2016 +0000
> # Node ID 8e614304b040537dbc735acb6c3999a05efd258c
> # Parent  011122b3b1c42374fb0489d107418e1be3665ca6
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 8e614304b040
> pager: wrap ui._runpager

Queued, thanks.

>
> As discussed at [1], ui._runpager will be the new low-level API accepting a
> pager command to actually run the pager. And ui.pager is the high-level API
> which reads config directly from self.
>
> This change is necessary for chgserver to override _runpager cleanly.
>
> [1]: www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091656.html
>
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -119,4 +119,10 @@ def uisetup(ui):
>          return
>
> +    class pagerui(ui.__class__):
> +        def _runpager(self, pagercmd):
> +            _runpager(self, pagercmd)
> +
> +    ui.__class__ = pagerui
> +
>      # chg has its own pager implementation
>      argv = sys.argv[:]
> @@ -158,5 +164,5 @@ def uisetup(ui):
>              if util.safehasattr(signal, "SIGPIPE"):
>                  signal.signal(signal.SIGPIPE, signal.SIG_DFL)
> -            _runpager(ui, p)
> +            ui._runpager(p)
>          return orig(ui, options, cmd, cmdfunc)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Jan. 8, 2017, 12:02 a.m.
On Fri, 6 Jan 2017 17:08:24 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1482711944 0
> #      Mon Dec 26 00:25:44 2016 +0000
> # Node ID 8e614304b040537dbc735acb6c3999a05efd258c
> # Parent  011122b3b1c42374fb0489d107418e1be3665ca6
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 8e614304b040
> pager: wrap ui._runpager
> 
> As discussed at [1], ui._runpager will be the new low-level API accepting a
> pager command to actually run the pager. And ui.pager is the high-level API
> which reads config directly from self.
> 
> This change is necessary for chgserver to override _runpager cleanly.
> 
> [1]: www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091656.html
> 
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -119,4 +119,10 @@ def uisetup(ui):
>          return
>  
> +    class pagerui(ui.__class__):
> +        def _runpager(self, pagercmd):
> +            _runpager(self, pagercmd)
> +
> +    ui.__class__ = pagerui

The change looks okay, but I should note that there might be a uisetup() battle
if we're planning to delay uisetup() and extsetup() until the 2nd dispatch()
called by chgserver.runcommand(), where ui._runpager() would be overridden by
chg beforehand.
Jun Wu - Jan. 9, 2017, 5:04 a.m.
Excerpts from Yuya Nishihara's message of 2017-01-08 09:02:13 +0900:
> On Fri, 6 Jan 2017 17:08:24 +0000, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1482711944 0
> > #      Mon Dec 26 00:25:44 2016 +0000
> > # Node ID 8e614304b040537dbc735acb6c3999a05efd258c
> > # Parent  011122b3b1c42374fb0489d107418e1be3665ca6
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 8e614304b040
> > pager: wrap ui._runpager
> > 
> > As discussed at [1], ui._runpager will be the new low-level API accepting a
> > pager command to actually run the pager. And ui.pager is the high-level API
> > which reads config directly from self.
> > 
> > This change is necessary for chgserver to override _runpager cleanly.
> > 
> > 
> > diff --git a/hgext/pager.py b/hgext/pager.py
> > --- a/hgext/pager.py
> > +++ b/hgext/pager.py
> > @@ -119,4 +119,10 @@ def uisetup(ui):
> >          return
> >  
> > +    class pagerui(ui.__class__):
> > +        def _runpager(self, pagercmd):
> > +            _runpager(self, pagercmd)
> > +
> > +    ui.__class__ = pagerui
> 
> The change looks okay, but I should note that there might be a uisetup() battle
> if we're planning to delay uisetup() and extsetup() until the 2nd dispatch()
> called by chgserver.runcommand(), where ui._runpager() would be overridden by
> chg beforehand.

The plan is as follows:

  1. For the current situation (pager is not in core),
     pager will be aware of chg wrapped ui - which means pager will not
     wrap ui if it sees hasattr(ui, '_runpager').
  2. In the future, when _runpager is defined in ui.py, no special if
     condition will be needed.
Yuya Nishihara - Jan. 9, 2017, 12:41 p.m.
On Mon, 9 Jan 2017 05:04:21 +0000, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-01-08 09:02:13 +0900:
> > On Fri, 6 Jan 2017 17:08:24 +0000, Jun Wu wrote:
> > > # HG changeset patch
> > > # User Jun Wu <quark@fb.com>
> > > # Date 1482711944 0
> > > #      Mon Dec 26 00:25:44 2016 +0000
> > > # Node ID 8e614304b040537dbc735acb6c3999a05efd258c
> > > # Parent  011122b3b1c42374fb0489d107418e1be3665ca6
> > > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 8e614304b040
> > > pager: wrap ui._runpager
> > > 
> > > As discussed at [1], ui._runpager will be the new low-level API accepting a
> > > pager command to actually run the pager. And ui.pager is the high-level API
> > > which reads config directly from self.
> > > 
> > > This change is necessary for chgserver to override _runpager cleanly.
> > > 
> > > 
> > > diff --git a/hgext/pager.py b/hgext/pager.py
> > > --- a/hgext/pager.py
> > > +++ b/hgext/pager.py
> > > @@ -119,4 +119,10 @@ def uisetup(ui):
> > >          return
> > >  
> > > +    class pagerui(ui.__class__):
> > > +        def _runpager(self, pagercmd):
> > > +            _runpager(self, pagercmd)
> > > +
> > > +    ui.__class__ = pagerui
> > 
> > The change looks okay, but I should note that there might be a uisetup() battle
> > if we're planning to delay uisetup() and extsetup() until the 2nd dispatch()
> > called by chgserver.runcommand(), where ui._runpager() would be overridden by
> > chg beforehand.
> 
> The plan is as follows:
> 
>   1. For the current situation (pager is not in core),
>      pager will be aware of chg wrapped ui - which means pager will not
>      wrap ui if it sees hasattr(ui, '_runpager').
>   2. In the future, when _runpager is defined in ui.py, no special if
>      condition will be needed.

Ah, sounds good.

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -119,4 +119,10 @@  def uisetup(ui):
         return
 
+    class pagerui(ui.__class__):
+        def _runpager(self, pagercmd):
+            _runpager(self, pagercmd)
+
+    ui.__class__ = pagerui
+
     # chg has its own pager implementation
     argv = sys.argv[:]
@@ -158,5 +164,5 @@  def uisetup(ui):
             if util.safehasattr(signal, "SIGPIPE"):
                 signal.signal(signal.SIGPIPE, signal.SIG_DFL)
-            _runpager(ui, p)
+            ui._runpager(p)
         return orig(ui, options, cmd, cmdfunc)