changeset 31108:3f8f53190d6a

chg: deduplicate error handling of ui.system() This moves 'onerr' handling from low-level util.system() to higher level, which seems better API separation.
author Yuya Nishihara <yuya@tcha.org>
date Sun, 19 Feb 2017 01:16:45 +0900
parents fbce78c58f1e
children 53230c5bb273
files mercurial/chgserver.py mercurial/ui.py mercurial/util.py tests/test-config.t
diffstat 4 files changed, 23 insertions(+), 27 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/chgserver.py	Sun Feb 19 01:00:10 2017 +0900
+++ b/mercurial/chgserver.py	Sun Feb 19 01:16:45 2017 +0900
@@ -179,7 +179,7 @@
             else:
                 self._csystem = csystem
 
-        def _runsystem(self, cmd, environ, cwd, onerr, errprefix, out):
+        def _runsystem(self, cmd, environ, cwd, out):
             # fallback to the original system method if the output needs to be
             # captured (to self._buffers), or the output stream is not stdout
             # (e.g. stderr, cStringIO), because the chg client is not aware of
@@ -187,17 +187,9 @@
             if (out is not self.fout
                 or not util.safehasattr(self.fout, 'fileno')
                 or self.fout.fileno() != util.stdout.fileno()):
-                return util.system(cmd, environ=environ, cwd=cwd, onerr=onerr,
-                                   errprefix=errprefix, out=out)
+                return util.system(cmd, environ=environ, cwd=cwd, out=out)
             self.flush()
-            rc = self._csystem(cmd, util.shellenviron(environ), cwd)
-            if rc and onerr:
-                errmsg = '%s %s' % (os.path.basename(cmd.split(None, 1)[0]),
-                                    util.explainexit(rc)[0])
-                if errprefix:
-                    errmsg = '%s: %s' % (errprefix, errmsg)
-                raise onerr(errmsg)
-            return rc
+            return self._csystem(cmd, util.shellenviron(environ), cwd)
 
         def _runpager(self, cmd):
             self._csystem(cmd, util.shellenviron(), type='pager',
--- a/mercurial/ui.py	Sun Feb 19 01:00:10 2017 +0900
+++ b/mercurial/ui.py	Sun Feb 19 01:16:45 2017 +0900
@@ -1281,6 +1281,9 @@
                blockedtag=None):
         '''execute shell command with appropriate output stream. command
         output will be redirected if fout is not stdout.
+
+        if command fails and onerr is None, return status, else raise onerr
+        object as exception.
         '''
         if blockedtag is None:
             blockedtag = 'unknown_system_' + cmd.translate(None, _keepalnum)
@@ -1288,14 +1291,19 @@
         if any(s[1] for s in self._bufferstates):
             out = self
         with self.timeblockedsection(blockedtag):
-            return self._runsystem(cmd, environ=environ, cwd=cwd, onerr=onerr,
-                                   errprefix=errprefix, out=out)
+            rc = self._runsystem(cmd, environ=environ, cwd=cwd, out=out)
+        if rc and onerr:
+            errmsg = '%s %s' % (os.path.basename(cmd.split(None, 1)[0]),
+                                util.explainexit(rc)[0])
+            if errprefix:
+                errmsg = '%s: %s' % (errprefix, errmsg)
+            raise onerr(errmsg)
+        return rc
 
-    def _runsystem(self, cmd, environ, cwd, onerr, errprefix, out):
+    def _runsystem(self, cmd, environ, cwd, out):
         """actually execute the given shell command (can be overridden by
         extensions like chg)"""
-        return util.system(cmd, environ=environ, cwd=cwd, onerr=onerr,
-                           errprefix=errprefix, out=out)
+        return util.system(cmd, environ=environ, cwd=cwd, out=out)
 
     def traceback(self, exc=None, force=False):
         '''print exception traceback if traceback printing enabled or forced.
--- a/mercurial/util.py	Sun Feb 19 01:00:10 2017 +0900
+++ b/mercurial/util.py	Sun Feb 19 01:16:45 2017 +0900
@@ -1009,20 +1009,16 @@
     env['HG'] = hgexecutable()
     return env
 
-def system(cmd, environ=None, cwd=None, onerr=None, errprefix=None, out=None):
+def system(cmd, environ=None, cwd=None, out=None):
     '''enhanced shell command execution.
     run with environment maybe modified, maybe in different dir.
 
-    if command fails and onerr is None, return status, else raise onerr
-    object as exception.
-
     if out is specified, it is assumed to be a file-like object that has a
     write() method. stdout and stderr will be redirected to out.'''
     try:
         stdout.flush()
     except Exception:
         pass
-    origcmd = cmd
     cmd = quotecommand(cmd)
     if pycompat.sysplatform == 'plan9' and (sys.version_info[0] == 2
                                     and sys.version_info[1] < 7):
@@ -1046,12 +1042,6 @@
             rc = proc.returncode
         if pycompat.sysplatform == 'OpenVMS' and rc & 1:
             rc = 0
-    if rc and onerr:
-        errmsg = '%s %s' % (os.path.basename(origcmd.split(None, 1)[0]),
-                            explainexit(rc)[0])
-        if errprefix:
-            errmsg = '%s: %s' % (errprefix, errmsg)
-        raise onerr(errmsg)
     return rc
 
 def checksignature(func):
--- a/tests/test-config.t	Sun Feb 19 01:00:10 2017 +0900
+++ b/tests/test-config.t	Sun Feb 19 01:16:45 2017 +0900
@@ -158,3 +158,9 @@
   $ hg showconfig paths
   paths.foo:suboption=~/foo
   paths.foo=$TESTTMP/foo
+
+edit failure
+
+  $ HGEDITOR=false hg config --edit
+  abort: edit failed: false exited with status 1
+  [255]