ui:util:pager: cleanup pager implementation

Changes:

* Import libraries with an underscore prefix to avoid polluting the
  module's public namespace.
* Use a copy `env` to avoid messing with the child process'
  environment.
* Pass the tweaked environment on to the PAGER (e.g. for PATH).
* Handle PAGER='' (by not paging).
* Use shlex.split to handle cases like PAGER='less -FRSX' (thanks to
  Da_Blitz from pocketnix.org for the bug report and shlex idea).

diff --git a/libbe/ui/util/pager.py b/libbe/ui/util/pager.py
index 20a583e2f40c472970370eec92b4e105477ca9c6..2d767a04492fca823b87626109760df0d52a899a 100644 (file)
--- a/libbe/ui/util/pager.py
+++ b/libbe/ui/util/pager.py
@@ -16,52 +16,69 @@
 # You should have received a copy of the GNU General Public License along with
 # Bugs Everywhere.  If not, see <http://www.gnu.org/licenses/>.
 
+"""Automatic pager for terminal output (a la Git).
 """
-Automatic pager for terminal output (a la Git).
-"""
 
-import sys, os, select
+import os as _os
+import select as _select
+import shlex as _shlex
+import sys as _sys
+
 
 # Inspired by Nathan Weizenbaum's
 #   http://nex-3.com/posts/73-git-style-automatic-paging-in-ruby
 def run_pager(paginate='auto'):
-    """
+    """Use the environment variable PAGER page future stdout output
+
     paginate should be one of 'never', 'auto', or 'always'.
 
     usage: just call this function and continue using sys.stdout like
     you normally would.
+
+    Notes
+    -----
+
+    This function creates forks a child, which continues executing the
+    calling code.  The parent calls :py:func:`os.execvpe` to morph
+    into `PAGER`.  The child keeps the original stdin, and the child's
+    stdout becomes the parent's stdin.  The parent keeps the original
+    stdout.
     """
-    if paginate == 'never' \
-            or sys.platform == 'win32' \
-            or not hasattr(sys.stdout, 'isatty') \
-            or sys.stdout.isatty() == False:
+    if (paginate == 'never' or
+        _sys.platform == 'win32' or
+        not hasattr(_sys.stdout, 'isatty') or
+        not _sys.stdout.isatty()):
         return
 
+    env = dict(_os.environ)
     if paginate == 'auto':
-        if 'LESS' not in os.environ:
-            os.environ['LESS'] = '' # += doesn't work on undefined var
+        if 'LESS' not in env:
+            env['LESS'] = ''  # += doesn't work on undefined var
+        else:
+            env['LESS'] += ' '  # separate from existing variables
         # don't page if the input is short enough
-        os.environ['LESS'] += ' -FRX'
-    if 'PAGER' in os.environ:
-        pager = os.environ['PAGER']
-    else:
-        pager = 'less'
+        env['LESS'] += '-FRX'
+    pager = _os.environ.get('PAGER', 'less')
+    args = _shlex.split(pager)
+    pager = args[0]
+    if not pager:  # handle PAGER=''
+        return
 
-    read_fd, write_fd = os.pipe()
-    if os.fork() == 0:
-        # child process
-        os.close(read_fd)
-        os.dup2(write_fd, 1)
-        os.close(write_fd)
-        if hasattr(sys.stderr, 'isatty') and sys.stderr.isatty() == True:
-            os.dup2(1, 2)
+    read_fd, write_fd = _os.pipe()
+    if _os.fork() == 0:
+        # child process, keep executing Python program
+        _os.close(read_fd)
+        _os.dup2(write_fd, 1)
+        _os.close(write_fd)
+        if hasattr(_sys.stderr, 'isatty') and _sys.stderr.isatty():
+            _os.dup2(1, 2)
         return
 
     # parent process, become pager
-    os.close(write_fd)
-    os.dup2(read_fd, 0)
-    os.close(read_fd)
+    _os.close(write_fd)
+    _os.dup2(read_fd, 0)
+    _os.close(read_fd)
 
     # Wait until we have input before we start the pager
-    select.select([0], [], [])
-    os.execlp(pager, pager)
+    _select.select([0], [], [])
+    _os.execvpe(pager, args, env)