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

login
register
mail settings
Submitter Jun Wu
Date March 14, 2016, 8:33 p.m.
Message ID <d931d9d34b74ae2d4ed1.1457987629@x1c>
Download mbox | patch
Permalink /patch/13886/
State Superseded
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 14, 2016, 8:33 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457967799 0
#      Mon Mar 14 15:03:19 2016 +0000
# Node ID d931d9d34b74ae2d4ed16737c98bfe6b56ec79e5
# Parent  e14741b16063879d0d4d67a4ad5f0129b8f4f769
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.
Sean Farley - March 14, 2016, 8:37 p.m.
Jun Wu <quark@fb.com> writes:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457967799 0
> #      Mon Mar 14 15:03:19 2016 +0000
> # Node ID d931d9d34b74ae2d4ed16737c98bfe6b56ec79e5
> # Parent  e14741b16063879d0d4d67a4ad5f0129b8f4f769
> 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,10 @@
>      if '--debugger' in sys.argv or not ui.formatted():
>          return
>  
> +    # chg has its own pager implementation
> +    if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv):
> +        return
> +

Would this be reason enough to put pager in core so we wouldn't need
different implementations?
Jun Wu - March 14, 2016, 8:49 p.m.
On 03/14/2016 08:37 PM, Sean Farley wrote:
> Would this be reason enough to put pager in core so we wouldn't need
> different implementations?

The reason that chg needs a different implementation is the pager must be
executed by the client. Otherwise it's likely to have trouble with
"terminal foreground job", Ctrl+Z etc.

To be more precise, the pager is currently using subprocess.Popen to create
process at server side. chg needs it to be "remotely" executed at client
side.

I think Yuya has plans to unify the implementations somehow. See
https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-March/081393.html
Sean Farley - March 14, 2016, 8:53 p.m.
Jun Wu <quark@fb.com> writes:

> On 03/14/2016 08:37 PM, Sean Farley wrote:
>> Would this be reason enough to put pager in core so we wouldn't need
>> different implementations?
>
> The reason that chg needs a different implementation is the pager must be
> executed by the client. Otherwise it's likely to have trouble with
> "terminal foreground job", Ctrl+Z etc.
>
> To be more precise, the pager is currently using subprocess.Popen to create
> process at server side. chg needs it to be "remotely" executed at client
> side.
>
> I think Yuya has plans to unify the implementations somehow. See
> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-March/081393.html

Yes, ok, this is more of what I was hoping (thanks Yuya for the ideas).
Jun Wu - March 15, 2016, 1:26 a.m.
On 03/14/2016 08:33 PM, Jun Wu wrote:
> +    if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv):

Sorry but this is incorrect. _earlygetopt requires an array. It should be:

 > +    if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], sys.argv):
Yuya Nishihara - March 15, 2016, 4:10 p.m.
On Tue, 15 Mar 2016 01:26:10 +0000, Jun Wu wrote:
> On 03/14/2016 08:33 PM, Jun Wu wrote:
> > +    if 'chgunix' in dispatch._earlygetopt('--cmdserver', sys.argv):
> 
> Sorry but this is incorrect. _earlygetopt requires an array. It should be:
> 
>  > +    if 'chgunix' in dispatch._earlygetopt(['--cmdserver'], sys.argv):

and _earlygetopt() modifies sys.argv, which is wrong.

Patch

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