Patchwork [3,of,8,py3,v3] extensions: tapdance to get reasonable import error formatting

login
register
mail settings
Submitter Augie Fackler
Date March 8, 2017, 11:22 p.m.
Message ID <bacac69ec34b5ee25298.1489015362@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/19033/
State Accepted
Headers show

Comments

Augie Fackler - March 8, 2017, 11:22 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1488568082 18000
#      Fri Mar 03 14:08:02 2017 -0500
# Node ID bacac69ec34b5ee252988003ee5f77ebc328f67d
# Parent  009c8aeb34d4f226ba15c54b37dbe8fb86cc103b
extensions: tapdance to get reasonable import error formatting

I'm not thrilled with this, but it seems to work.
Yuya Nishihara - March 9, 2017, 7:48 a.m.
On Wed, 08 Mar 2017 18:22:42 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1488568082 18000
> #      Fri Mar 03 14:08:02 2017 -0500
> # Node ID bacac69ec34b5ee252988003ee5f77ebc328f67d
> # Parent  009c8aeb34d4f226ba15c54b37dbe8fb86cc103b
> extensions: tapdance to get reasonable import error formatting
> 
> I'm not thrilled with this, but it seems to work.
> 
> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -18,6 +18,7 @@ from .i18n import (
>  
>  from . import (
>      cmdutil,
> +    encoding,
>      error,
>      pycompat,
>      util,
> @@ -104,11 +105,19 @@ def _importext(name, path=None, reportfu
>                  mod = _importh(name)
>      return mod
>  
> +def _forbytes(inst):
> +    """Portably format an import error into a form suitable for
> +    %-formatting into bytestrings."""
> +    if pycompat.ispy3:
> +        return str(inst).encode('utf-8')
> +    return inst
> +
>  def _reportimporterror(ui, err, failed, next):
>      # note: this ui.debug happens before --debug is processed,
>      #       Use --config ui.debug=1 to see them.
> -    ui.debug('could not import %s (%s): trying %s\n'
> -             % (failed, err, next))
> +    msg = 'could not import %s (%s): trying %s\n' % (
> +        failed, _forbytes(err), next)
> +    ui.debug(encoding.tolocal(msg))

This tolocal() is okay since the message template is in ascii, but ...

> @@ -168,12 +177,13 @@ def loadall(ui):
>          except KeyboardInterrupt:
>              raise
>          except Exception as inst:
> +            inst = _forbytes(inst)
>              if path:
> -                ui.warn(_("*** failed to import extension %s from %s: %s\n")
> -                        % (name, path, inst))
> +                fmt = _("*** failed to import extension %s from %s: %s\n")
> +                ui.warn(encoding.tolocal(fmt % (name, path, inst)))
>              else:
> -                ui.warn(_("*** failed to import extension %s: %s\n")
> -                        % (name, inst))
> +                fmt = _("*** failed to import extension %s: %s\n")
> +                ui.warn(encoding.tolocal(fmt % (name, inst)))

These aren't.

Instead, _forbytes() could return a bytes processed by tolocal().

I've queued the first two, thanks.

Patch

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -18,6 +18,7 @@  from .i18n import (
 
 from . import (
     cmdutil,
+    encoding,
     error,
     pycompat,
     util,
@@ -104,11 +105,19 @@  def _importext(name, path=None, reportfu
                 mod = _importh(name)
     return mod
 
+def _forbytes(inst):
+    """Portably format an import error into a form suitable for
+    %-formatting into bytestrings."""
+    if pycompat.ispy3:
+        return str(inst).encode('utf-8')
+    return inst
+
 def _reportimporterror(ui, err, failed, next):
     # note: this ui.debug happens before --debug is processed,
     #       Use --config ui.debug=1 to see them.
-    ui.debug('could not import %s (%s): trying %s\n'
-             % (failed, err, next))
+    msg = 'could not import %s (%s): trying %s\n' % (
+        failed, _forbytes(err), next)
+    ui.debug(encoding.tolocal(msg))
     if ui.debugflag:
         ui.traceback()
 
@@ -168,12 +177,13 @@  def loadall(ui):
         except KeyboardInterrupt:
             raise
         except Exception as inst:
+            inst = _forbytes(inst)
             if path:
-                ui.warn(_("*** failed to import extension %s from %s: %s\n")
-                        % (name, path, inst))
+                fmt = _("*** failed to import extension %s from %s: %s\n")
+                ui.warn(encoding.tolocal(fmt % (name, path, inst)))
             else:
-                ui.warn(_("*** failed to import extension %s: %s\n")
-                        % (name, inst))
+                fmt = _("*** failed to import extension %s: %s\n")
+                ui.warn(encoding.tolocal(fmt % (name, inst)))
             ui.traceback()
 
     for name in _order[newindex:]: