Patchwork [STABLE] win32mbcs: make pycompat.bytestr return byte-str for color effect int value

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 22, 2017, 4:35 p.m.
Message ID <39457bf6db5f7c546ab6.1495470941@speaknoevil>
Download mbox | patch
Permalink /patch/20825/
State Changes Requested
Headers show

Comments

Katsunori FUJIWARA - May 22, 2017, 4:35 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1495466652 -32400
#      Tue May 23 00:24:12 2017 +0900
# Branch stable
# Node ID 39457bf6db5f7c546ab6d5a75e33ae6ded5fc0dd
# Parent  99515353c72a4c54e4aac1a2ad4f8f724c7fdc9c
win32mbcs: make pycompat.bytestr return byte-str for color effect int value

Since 176ed32dc159, pycompat.bytestr() wrapped by win32mbcs returns
unicode object, if an argument is not byte-str object. And this causes
unexpected failure at colorization.

pycompat.bytestr() is used to convert from color effect "int" value to
byte-str object in color module. Wrapped pycompat.bytestr() returns
unicode object for such "int" value, because it isn't byte-str.

If this returned unicode object is used to colorize non-ASCII byte-str
in cases below, UnicodeDecodeError is raised at an operation between
them.

  - colorization uses "ansi" color mode, or

    Even though this isn't default on Windows, user might use this
    color mode for third party pager.

  - ui.write() is buffered with labeled=True

    Buffering causes "ansi" color mode internally, regardless of
    actual color mode. With "win32" color mode, extra escape sequences
    are omitted at writing data out.

    For example, with "win32" color mode, "hg status" doesn't fail for
    non-ASCII filenames, but "hg log" does for non-ASCII text, because
    the latter implies buffered formatter.

There are many "color effect" value lines in color.py, and making them
byte-str objects isn't suitable for fixing on stable.

To make pycompat.bytestr() return byte-str for color effect int value,
this patch passes "(str, int)" tuple instead of "str" to
basewrapper(). This tuple is used as the second argument of
"isinstance()" at examination of arguments, and prevents wrapped
functions from returning unicode object for "int" argument.
Yuya Nishihara - May 24, 2017, 2:54 p.m.
On Tue, 23 May 2017 01:35:41 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1495466652 -32400
> #      Tue May 23 00:24:12 2017 +0900
> # Branch stable
> # Node ID 39457bf6db5f7c546ab6d5a75e33ae6ded5fc0dd
> # Parent  99515353c72a4c54e4aac1a2ad4f8f724c7fdc9c
> win32mbcs: make pycompat.bytestr return byte-str for color effect int value

bytestr() may accept arbitrary objects that can be converted to str. Perhaps
we'll need to add a stub function (say util._filenamebytestr = bytestr) and
wrap it instead?
Katsunori FUJIWARA - May 24, 2017, 4:26 p.m.
At Wed, 24 May 2017 23:54:55 +0900,
Yuya Nishihara wrote:
> 
> On Tue, 23 May 2017 01:35:41 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1495466652 -32400
> > #      Tue May 23 00:24:12 2017 +0900
> > # Branch stable
> > # Node ID 39457bf6db5f7c546ab6d5a75e33ae6ded5fc0dd
> > # Parent  99515353c72a4c54e4aac1a2ad4f8f724c7fdc9c
> > win32mbcs: make pycompat.bytestr return byte-str for color effect int value
> 
> bytestr() may accept arbitrary objects that can be converted to str. Perhaps
> we'll need to add a stub function (say util._filenamebytestr = bytestr) and
> wrap it instead?
> 

OK, I'll revise this.

BTW, is "_" prefix needed for stub name ?
Yuya Nishihara - May 25, 2017, 1:34 p.m.
On Thu, 25 May 2017 01:26:10 +0900, FUJIWARA Katsunori wrote:
> At Wed, 24 May 2017 23:54:55 +0900,
> Yuya Nishihara wrote:
> > 
> > On Tue, 23 May 2017 01:35:41 +0900, FUJIWARA Katsunori wrote:
> > > # HG changeset patch
> > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > > # Date 1495466652 -32400
> > > #      Tue May 23 00:24:12 2017 +0900
> > > # Branch stable
> > > # Node ID 39457bf6db5f7c546ab6d5a75e33ae6ded5fc0dd
> > > # Parent  99515353c72a4c54e4aac1a2ad4f8f724c7fdc9c
> > > win32mbcs: make pycompat.bytestr return byte-str for color effect int value
> > 
> > bytestr() may accept arbitrary objects that can be converted to str. Perhaps
> > we'll need to add a stub function (say util._filenamebytestr = bytestr) and
> > wrap it instead?
> > 
> 
> OK, I'll revise this.

Thanks.

> BTW, is "_" prefix needed for stub name ?

I prefer a prefixed name since it's just a workaround for win32mbcs.

Patch

diff --git a/hgext/win32mbcs.py b/hgext/win32mbcs.py
--- a/hgext/win32mbcs.py
+++ b/hgext/win32mbcs.py
@@ -121,7 +121,11 @@  def wrapper(func, args, kwds):
 
 
 def reversewrapper(func, args, kwds):
-    return basewrapper(func, str, decode, encode, args, kwds)
+    # "int" is checked for pycompat.bytestr() implied in
+    # mercurial.color._render_effects(). This examination should be
+    # safe enough, because all reverse-wrapped functions are always
+    # invoked with only one argument.
+    return basewrapper(func, (str, int), decode, encode, args, kwds)
 
 def wrapperforlistdir(func, args, kwds):
     # Ensure 'path' argument ends with os.sep to avoids