Patchwork [RFC] pager: migrate to core

login
register
mail settings
Submitter Bryan O'Sullivan
Date Nov. 24, 2015, 6:31 p.m.
Message ID <f63938a2292eaec78637.1448389874@bryano-mbp.local>
Download mbox | patch
Permalink /patch/11604/
State Superseded
Headers show

Comments

Bryan O'Sullivan - Nov. 24, 2015, 6:31 p.m.
# HG changeset patch
# User Bryan O'Sullivan <bos@serpentine.com>
# Date 1448389126 28800
#      Tue Nov 24 10:18:46 2015 -0800
# Node ID f63938a2292eaec78637835221916794d6f60994
# Parent  f668eef04abc3de94f41b527daa0bb7a0cf76f56
pager: migrate to core

We need to retain an extension module for a long time (possibly
indefinitely), for two reasons:

- Deleting the extension entirely will break the configs of existing
  users of the extension.

- Some out-of-tree extensions "know" that hgext.pager.attended is
  a variable they can modify.
Matt Mackall - Nov. 24, 2015, 9:44 p.m.
On Tue, 2015-11-24 at 10:31 -0800, Bryan O'Sullivan wrote:
> # HG changeset patch
> # User Bryan O'Sullivan <bos@serpentine.com>
> # Date 1448389126 28800
> #      Tue Nov 24 10:18:46 2015 -0800
> # Node ID f63938a2292eaec78637835221916794d6f60994
> # Parent  f668eef04abc3de94f41b527daa0bb7a0cf76f56
> pager: migrate to core
> 
> We need to retain an extension module for a long time (possibly
> indefinitely), for two reasons:
> 
> - Deleting the extension entirely will break the configs of existing
>   users of the extension.

This is what the _ignore list in extensions.py is for. We've eliminated
half a dozen extensions without breaking config files.

> - Some out-of-tree extensions "know" that hgext.pager.attended is
>   a variable they can modify.

It's our long-standing policy to break them and shed no tears.

Seriously, we can't let ourselves be blocked from changing internals
that random third-party extensions have or may have latched onto. There
are intentionally no limits on what an extension can touch, wrap, or
replace and that's actually what makes many extensions possible. And if
that means we can't change anything for fear of breaking someone else's
extension, it means development on the core stops. So since the
beginning, I've said the price of having absolutely free reign in
extensions is (a) you're a GPL derived work and (b) you're responsible
for keeping up with our changes.

(And in this particular case, I strongly suspect these extensions are
actually second-party.)

-- 
Mathematics is the supreme nostalgia of our time.
Bryan O'Sullivan - Nov. 24, 2015, 10:06 p.m.
On Tue, Nov 24, 2015 at 4:44 PM, Matt Mackall <mpm@selenic.com> wrote:

> This is what the _ignore list in extensions.py is for. We've eliminated
> half a dozen extensions without breaking config files.
>
>
Brilliant, even better!
Daniel Colascione (SEATTLE) - Nov. 24, 2015, 10:11 p.m.
On 11/24/2015 02:06 PM, Bryan O'Sullivan wrote:
> 
> On Tue, Nov 24, 2015 at 4:44 PM, Matt Mackall <mpm@selenic.com
> <mailto:mpm@selenic.com>> wrote:
> 
>     This is what the _ignore list in extensions.py is for. We've eliminated
>     half a dozen extensions without breaking config files.
> 
> 
> Brilliant, even better!

Wouldn't that list better be called built_in_extensions, or
moved_to_core, or something? It's not clear from the source (at least
the version I just looked at) *why* those extensions are ignored.
Bryan O'Sullivan - Nov. 24, 2015, 10:22 p.m.
On Tue, Nov 24, 2015 at 5:11 PM, Daniel Colascione (SEATTLE) <dancol@fb.com>
wrote:

> Wouldn't that list better be called built_in_extensions, or
> moved_to_core, or something? It's not clear from the source (at least
> the version I just looked at) *why* those extensions are ignored.
>

Funny thing, I just sent a second patch with this in mind.

Patch

diff -r f668eef04abc -r f63938a2292e hgext/pager.py
--- a/hgext/pager.py	Fri Nov 20 16:54:55 2015 -0500
+++ b/hgext/pager.py	Tue Nov 24 10:18:46 2015 -0800
@@ -12,143 +12,9 @@ 
 #
 # Run "hg help pager" to get info on configuration.
 
-'''browse command output with an external pager
+'''now part of core mercurial, extension for compatibility only'''
 
-To set the pager that should be used, set the application variable::
+from mercurial import pager
 
-  [pager]
-  pager = less -FRX
-
-If no pager is set, the pager extensions uses the environment variable
-$PAGER. If neither pager.pager, nor $PAGER is set, no pager is used.
-
-You can disable the pager for certain commands by adding them to the
-pager.ignore list::
-
-  [pager]
-  ignore = version, help, update
-
-You can also enable the pager only for certain commands using
-pager.attend. Below is the default list of commands to be paged::
-
-  [pager]
-  attend = annotate, cat, diff, export, glog, log, qdiff
-
-Setting pager.attend to an empty value will cause all commands to be
-paged.
-
-If pager.attend is present, pager.ignore will be ignored.
-
-Lastly, you can enable and disable paging for individual commands with
-the attend-<command> option. This setting takes precedence over
-existing attend and ignore options and defaults::
-
-  [pager]
-  attend-cat = false
-
-To ignore global commands like :hg:`version` or :hg:`help`, you have
-to specify them in your user configuration file.
-
-The --pager=... option can also be used to control when the pager is
-used. Use a boolean value like yes, no, on, off, or use auto for
-normal behavior.
-
-'''
-
-import atexit, sys, os, signal, subprocess
-from mercurial import commands, dispatch, util, extensions, cmdutil
-from mercurial.i18n import _
-
-# Note for extension authors: ONLY specify testedwith = 'internal' for
-# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
-# be specifying the version(s) of Mercurial they are tested with, or
-# leave the attribute unspecified.
-testedwith = 'internal'
-
-def _runpager(ui, p):
-    pager = subprocess.Popen(p, shell=True, bufsize=-1,
-                             close_fds=util.closefds, stdin=subprocess.PIPE,
-                             stdout=sys.stdout, stderr=sys.stderr)
-
-    # back up original file objects and descriptors
-    olduifout = ui.fout
-    oldstdout = sys.stdout
-    stdoutfd = os.dup(sys.stdout.fileno())
-    stderrfd = os.dup(sys.stderr.fileno())
-
-    # create new line-buffered stdout so that output can show up immediately
-    ui.fout = sys.stdout = newstdout = os.fdopen(sys.stdout.fileno(), 'wb', 1)
-    os.dup2(pager.stdin.fileno(), sys.stdout.fileno())
-    if ui._isatty(sys.stderr):
-        os.dup2(pager.stdin.fileno(), sys.stderr.fileno())
-
-    @atexit.register
-    def killpager():
-        if util.safehasattr(signal, "SIGINT"):
-            signal.signal(signal.SIGINT, signal.SIG_IGN)
-        pager.stdin.close()
-        ui.fout = olduifout
-        sys.stdout = oldstdout
-        # close new stdout while it's associated with pager; otherwise stdout
-        # fd would be closed when newstdout is deleted
-        newstdout.close()
-        # restore original fds: stdout is open again
-        os.dup2(stdoutfd, sys.stdout.fileno())
-        os.dup2(stderrfd, sys.stderr.fileno())
-        pager.wait()
-
-def uisetup(ui):
-    if '--debugger' in sys.argv or not ui.formatted():
-        return
-
-    def pagecmd(orig, ui, options, cmd, cmdfunc):
-        p = ui.config("pager", "pager", os.environ.get("PAGER"))
-        usepager = False
-        always = util.parsebool(options['pager'])
-        auto = options['pager'] == 'auto'
-
-        if not p:
-            pass
-        elif always:
-            usepager = True
-        elif not auto:
-            usepager = False
-        else:
-            attend = ui.configlist('pager', 'attend', attended)
-            ignore = ui.configlist('pager', 'ignore')
-            cmds, _ = cmdutil.findcmd(cmd, commands.table)
-
-            for cmd in cmds:
-                var = 'attend-%s' % cmd
-                if ui.config('pager', var):
-                    usepager = ui.configbool('pager', var)
-                    break
-                if (cmd in attend or
-                     (cmd not in ignore and not attend)):
-                    usepager = True
-                    break
-
-        setattr(ui, 'pageractive', usepager)
-
-        if usepager:
-            ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
-            ui.setconfig('ui', 'interactive', False, 'pager')
-            if util.safehasattr(signal, "SIGPIPE"):
-                signal.signal(signal.SIGPIPE, signal.SIG_DFL)
-            _runpager(ui, p)
-        return orig(ui, options, cmd, cmdfunc)
-
-    # Wrap dispatch._runcommand after color is loaded so color can see
-    # ui.pageractive. Otherwise, if we loaded first, color's wrapped
-    # dispatch._runcommand would run without having access to ui.pageractive.
-    def afterloaded(loaded):
-        extensions.wrapfunction(dispatch, '_runcommand', pagecmd)
-    extensions.afterloaded('color', afterloaded)
-
-def extsetup(ui):
-    commands.globalopts.append(
-        ('', 'pager', 'auto',
-         _("when to paginate (boolean, always, auto, or never)"),
-         _('TYPE')))
-
-attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff']
+# needed by some out-of-tree extensions
+attended = pager.attended
diff -r f668eef04abc -r f63938a2292e mercurial/commands.py
--- a/mercurial/commands.py	Fri Nov 20 16:54:55 2015 -0500
+++ b/mercurial/commands.py	Tue Nov 24 10:18:46 2015 -0800
@@ -74,6 +74,8 @@  globalopts = [
     ('', 'version', None, _('output version information and exit')),
     ('h', 'help', None, _('display help and exit')),
     ('', 'hidden', False, _('consider hidden changesets')),
+    ('', 'pager', 'auto',
+     _("when to paginate (boolean, always, auto, or never)"), _('TYPE')),
 ]
 
 dryrunopts = [('n', 'dry-run', None,
diff -r f668eef04abc -r f63938a2292e mercurial/dispatch.py
--- a/mercurial/dispatch.py	Fri Nov 20 16:54:55 2015 -0500
+++ b/mercurial/dispatch.py	Tue Nov 24 10:18:46 2015 -0800
@@ -33,6 +33,7 @@  from . import (
     fancyopts,
     hg,
     hook,
+    pager,
     ui as uimod,
     util,
 )
@@ -779,6 +780,7 @@  def _dispatch(req):
     if shellaliasfn:
         return shellaliasfn()
 
+    pager.uisetup(lui)
     # Configure extensions in phases: uisetup, extsetup, cmdtable, and
     # reposetup. Programs like TortoiseHg will call _dispatch several
     # times so we keep track of configured extensions in _loaded.
diff -r f668eef04abc -r f63938a2292e mercurial/help.py
--- a/mercurial/help.py	Fri Nov 20 16:54:55 2015 -0500
+++ b/mercurial/help.py	Tue Nov 24 10:18:46 2015 -0800
@@ -172,6 +172,7 @@  helptable = sorted([
     (["glossary"], _("Glossary"), loaddoc('glossary')),
     (["hgignore", "ignore"], _("Syntax for Mercurial Ignore Files"),
      loaddoc('hgignore')),
+    (["pager"], _("Using an External Pager"), loaddoc('pager')),
     (["phases"], _("Working with Phases"), loaddoc('phases')),
     (['scripting'], _('Using Mercurial from scripts and automation'),
      loaddoc('scripting')),
diff -r f668eef04abc -r f63938a2292e mercurial/help/pager.txt
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial/help/pager.txt	Tue Nov 24 10:18:46 2015 -0800
@@ -0,0 +1,38 @@ 
+To set the pager that should be used, set the application variable::
+
+  [pager]
+  pager = less -FRX
+
+If no pager is set, the pager extensions uses the environment variable
+$PAGER. If neither pager.pager, nor $PAGER is set, no pager is used.
+
+You can disable the pager for certain commands by adding them to the
+pager.ignore list::
+
+  [pager]
+  ignore = version, help, update
+
+You can also enable the pager only for certain commands using
+pager.attend. Below is the default list of commands to be paged::
+
+  [pager]
+  attend = annotate, cat, diff, export, glog, log, qdiff
+
+Setting pager.attend to an empty value will cause all commands to be
+paged.
+
+If pager.attend is present, pager.ignore will be ignored.
+
+Lastly, you can enable and disable paging for individual commands with
+the attend-<command> option. This setting takes precedence over
+existing attend and ignore options and defaults::
+
+  [pager]
+  attend-cat = false
+
+To ignore global commands like :hg:`version` or :hg:`help`, you have
+to specify them in your user configuration file.
+
+The --pager=... option can also be used to control when the pager is
+used. Use a boolean value like yes, no, on, off, or use auto for
+normal behavior.
diff -r f668eef04abc -r f63938a2292e mercurial/pager.py
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial/pager.py	Tue Nov 24 10:18:46 2015 -0800
@@ -0,0 +1,92 @@ 
+# pager.py - display output using a pager
+#
+# Copyright 2008 David Soria Parra <dsp@php.net>
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+import atexit, sys, os, signal, subprocess
+from . import commands, dispatch, util, extensions, cmdutil
+from .i18n import _
+
+def _runpager(ui, p):
+    pager = subprocess.Popen(p, shell=True, bufsize=-1,
+                             close_fds=util.closefds, stdin=subprocess.PIPE,
+                             stdout=sys.stdout, stderr=sys.stderr)
+
+    # back up original file objects and descriptors
+    olduifout = ui.fout
+    oldstdout = sys.stdout
+    stdoutfd = os.dup(sys.stdout.fileno())
+    stderrfd = os.dup(sys.stderr.fileno())
+
+    # create new line-buffered stdout so that output can show up immediately
+    ui.fout = sys.stdout = newstdout = os.fdopen(sys.stdout.fileno(), 'wb', 1)
+    os.dup2(pager.stdin.fileno(), sys.stdout.fileno())
+    if ui._isatty(sys.stderr):
+        os.dup2(pager.stdin.fileno(), sys.stderr.fileno())
+
+    @atexit.register
+    def killpager():
+        if util.safehasattr(signal, "SIGINT"):
+            signal.signal(signal.SIGINT, signal.SIG_IGN)
+        pager.stdin.close()
+        ui.fout = olduifout
+        sys.stdout = oldstdout
+        # close new stdout while it's associated with pager; otherwise stdout
+        # fd would be closed when newstdout is deleted
+        newstdout.close()
+        # restore original fds: stdout is open again
+        os.dup2(stdoutfd, sys.stdout.fileno())
+        os.dup2(stderrfd, sys.stderr.fileno())
+        pager.wait()
+
+def uisetup(ui):
+    if '--debugger' in sys.argv or not ui.formatted():
+        return
+
+    def pagecmd(orig, ui, options, cmd, cmdfunc):
+        p = ui.config("pager", "pager", os.environ.get("PAGER"))
+        usepager = False
+        always = util.parsebool(options['pager'])
+        auto = options['pager'] == 'auto'
+
+        if not p:
+            pass
+        elif always:
+            usepager = True
+        elif not auto:
+            usepager = False
+        else:
+            attend = ui.configlist('pager', 'attend', attended)
+            ignore = ui.configlist('pager', 'ignore')
+            cmds, _ = cmdutil.findcmd(cmd, commands.table)
+
+            for cmd in cmds:
+                var = 'attend-%s' % cmd
+                if ui.config('pager', var):
+                    usepager = ui.configbool('pager', var)
+                    break
+                if (cmd in attend or
+                     (cmd not in ignore and not attend)):
+                    usepager = True
+                    break
+
+        setattr(ui, 'pageractive', usepager)
+
+        if usepager:
+            ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
+            ui.setconfig('ui', 'interactive', False, 'pager')
+            if util.safehasattr(signal, "SIGPIPE"):
+                signal.signal(signal.SIGPIPE, signal.SIG_DFL)
+            _runpager(ui, p)
+        return orig(ui, options, cmd, cmdfunc)
+
+    # Wrap dispatch._runcommand after color is loaded so color can see
+    # ui.pageractive. Otherwise, if we loaded first, color's wrapped
+    # dispatch._runcommand would run without having access to ui.pageractive.
+    def aftercolor(loaded):
+        extensions.wrapfunction(dispatch, '_runcommand', pagecmd)
+    extensions.afterloaded('color', aftercolor)
+
+attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff']