Patchwork [V2] Improve error message when a user configured editor could not be found

login
register
mail settings
Submitter mw-u2@posteo.de
Date March 23, 2020, 9:34 a.m.
Message ID <aa5800cdabd69deca92e234a54d6ab71@posteo.de>
Download mbox | patch
Permalink /patch/45863/
State Accepted
Headers show

Comments

mw-u2@posteo.de - March 23, 2020, 9:34 a.m.
# HG changeset patch
# User Micha Wiedenmann
# Date 1584630384 -3600
#      Do Mrz 19 16:06:24 2020 +0100
# Node ID c8989a00cf3f4f19d621f075c900b2e5628c6acf
# Parent  ea40fea992e000fb32edcf41d57721601842a7f2
hg: Use "procutil.shellsplit" to parse command

A commandline containing a space ('"C:\\Program Files\\bar.exe" "..."') 
must not
simply split at whitespace, instead quoting has to be taken into 
account. Use
"shlex.split()" to parse it instead.

This can improve the error message if we fail to launch a user 
configured
editor which does not exist. Consider

     [ui]
     editor = "C:\Program Files\editor\editor.exe"

where the path does not exist. "hg histedit" currently aborts with

> Abort: edit failed: Program exited with status 1

here "Program" is not part of the message but the name of the program 
that
failed (i.e. `basename("C:\\Program ")`). With this change the message 
instead
reads

> Abort: edit failed: C:\Program Files\editor\editor.exe exited with 
> status 1

which is also not ideal since infact "cmd.exe" exited with code 1, not 
the
editor. But the real error message ("File not found") gets swallowed by
`procutil` and including the correct path improves the error message
nevertheless.

              if errprefix:
Yuya Nishihara - March 23, 2020, 10:54 a.m.
On Mon, 23 Mar 2020 10:34:01 +0100, mw-u2@posteo.de wrote:
> # HG changeset patch
> # User Micha Wiedenmann
> # Date 1584630384 -3600
> #      Do Mrz 19 16:06:24 2020 +0100
> # Node ID c8989a00cf3f4f19d621f075c900b2e5628c6acf
> # Parent  ea40fea992e000fb32edcf41d57721601842a7f2
> hg: Use "procutil.shellsplit" to parse command

Fixed whitespace errors and queued, thanks.

Please consider using the patchbomb extension or Phabricator.
https://www.mercurial-scm.org/wiki/ContributingChanges#Sending_patches

Patch

diff -r ea40fea992e0 -r c8989a00cf3f mercurial/ui.py
--- a/mercurial/ui.py	Do Mrz 12 16:25:22 2020 -0700
+++ b/mercurial/ui.py	Do Mrz 19 16:06:24 2020 +0100
@@ -1868,7 +1868,7 @@ 
              rc = self._runsystem(cmd, environ=environ, cwd=cwd, 
out=out)
          if rc and onerr:
              errmsg = b'%s %s' % (
-                os.path.basename(cmd.split(None, 1)[0]),
+                procutil.shellsplit(cmd)[0],
                  procutil.explainexit(rc),
              )