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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

mw-u2
Hi,

Summary: I ran "hg histedit" and got a strange error message which was
not helpful in tracking down the problem (I configured a non-existing
editor). This patch improves the error message.

Details: The error message of a failed attempt to launch the user
configured editor does not deal correctly with whitespace. Example
message when "C:\Program Files\editor\editor.exe" does not exist:

> Abort: edit failed: Program exited with status 1

Here "Program" is not part of the error message but
basename("C:\Program"). Instead it should have been "C:\Program
Files\editor\editor.exe" (or possible "editor.exe" which is much less
useful in tracking down the user error though).

The situation is still not ideal since "exited with status 1" refers to
the exit code of "cmd.exe" and the real error ("File not found") gets
swallowed in the process.

Regards,
Micha Wiedenmann

# HG changeset patch
# User Micha Wiedenmann
# Date 1584630384 -3600
#      Do Mrz 19 16:06:24 2020 +0100
# Node ID f66e7761209b675665a27765c2772eef697331bf
# Parent  ea40fea992e000fb32edcf41d57721601842a7f2
hg: Use "shlex()" to parse commandlines fowarded to "procutil.system()"

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.

diff -r ea40fea992e0 -r f66e7761209b 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
@@ -14,6 +14,7 @@
  import inspect
  import os
  import re
+import shlex
  import signal
  import socket
  import subprocess
@@ -1868,7 +1869,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]),
+                shlex.split(cmd)[0],
                  procutil.explainexit(rc),
              )
              if errprefix:


_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Improve error message when a user configured editor could not be found

Yuya Nishihara
On Thu, 19 Mar 2020 16:52:10 +0100, [hidden email] wrote:

> --- a/mercurial/ui.py Do Mrz 12 16:25:22 2020 -0700
> +++ b/mercurial/ui.py Do Mrz 19 16:06:24 2020 +0100
> @@ -14,6 +14,7 @@
>   import inspect
>   import os
>   import re
> +import shlex
>   import signal
>   import socket
>   import subprocess
> @@ -1868,7 +1869,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]),
> +                shlex.split(cmd)[0],

shlex has py2/3 compatibility issue. procutil.shellsplit() can be used
instead.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

mw-u2
# 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.

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),
              )
              if errprefix:


_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] Improve error message when a user configured editor could not be found

Yuya Nishihara
On Mon, 23 Mar 2020 10:34:01 +0100, [hidden email] 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
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel