[PATCH 3 of 4] ui: defer setting pager related properties until the pager has spawned

Matt Harbison mharbison72 at gmail.com
Sun Mar 26 00:41:06 EDT 2017


# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1490490720 14400
#      Sat Mar 25 21:12:00 2017 -0400
# Node ID 84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
# Parent  a263702b064a5a3ce1ca74b227e8e624e4b05874
ui: defer setting pager related properties until the pager has spawned

When --pager=on is given, dispatch.py spawns a pager before setting up color.
If the pager failed to launch, ui.pageractive was left set to True, so color
configured itself based on 'color.pagermode'.  A typical MSYS setting would be
'color.mode=auto, color.pagermode=ansi'.  In the failure case, this would print
a warning, disable the pager, and then print the raw ANSI codes to the terminal.

Care needs to be taken, because it appears that leaving ui.pageractive=True was
the only thing that prevented an attempt at running the pager again from inside
the command.  This results in a double warning message, so pager is simply
disabled on failure.

I don't understand what chg is doing with pager, or how it signals a failure to
spawn the pager.  But this is no worse that the previous behavior.  The ui
config settings didn't need to be moved to fix this, but it seemed like the
right thing to do for consistency.

diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -193,6 +193,7 @@
         def _runpager(self, cmd):
             self._csystem(cmd, util.shellenviron(), type='pager',
                           cmdtable={'attachio': attachio})
+            return True
 
     return chgui(srcui)
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -850,15 +850,22 @@
 
         self.debug('starting pager for command %r\n' % command)
         self.flush()
-        self.pageractive = True
-        # Preserve the formatted-ness of the UI. This is important
-        # because we mess with stdout, which might confuse
-        # auto-detection of things being formatted.
-        self.setconfig('ui', 'formatted', self.formatted(), 'pager')
-        self.setconfig('ui', 'interactive', False, 'pager')
+
+        wasformatted = self.formatted()
         if util.safehasattr(signal, "SIGPIPE"):
             signal.signal(signal.SIGPIPE, _catchterm)
-        self._runpager(pagercmd)
+        if self._runpager(pagercmd):
+            self.pageractive = True
+            # Preserve the formatted-ness of the UI. This is important
+            # because we mess with stdout, which might confuse
+            # auto-detection of things being formatted.
+            self.setconfig('ui', 'formatted', wasformatted, 'pager')
+            self.setconfig('ui', 'interactive', False, 'pager')
+        else:
+            # If the pager can't be spawned in dispatch when --pager=on is
+            # given, don't try again when the command runs, to avoid a duplicate
+            # warning about a missing pager command.
+            self.disablepager()
 
     def _runpager(self, command):
         """Actually start the pager and set up file descriptors.
@@ -868,7 +875,7 @@
         """
         if command == 'cat':
             # Save ourselves some work.
-            return
+            return False
         # If the command doesn't contain any of these characters, we
         # assume it's a binary and exec it directly. This means for
         # simple pager command configurations, we can degrade
@@ -898,7 +905,7 @@
             if e.errno == errno.ENOENT and not shell:
                 self.warn(_("missing pager command '%s', skipping pager\n")
                           % command)
-                return
+                return False
             raise
 
         # back up original file descriptors
@@ -919,6 +926,8 @@
             pager.stdin.close()
             pager.wait()
 
+        return True
+
     def interface(self, feature):
         """what interface to use for interactive console features?
 


More information about the Mercurial-devel mailing list