Patchwork [1,of,2] encoding: teach unitolocal() and unifromlocal() to handle None

login
register
mail settings
Submitter Matt Harbison
Date Sept. 23, 2018, 5:18 a.m.
Message ID <10b1e61357ac546e70e5.1537679932@Envy>
Download mbox | patch
Permalink /patch/34924/
State New
Headers show

Comments

Matt Harbison - Sept. 23, 2018, 5:18 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1537667864 14400
#      Sat Sep 22 21:57:44 2018 -0400
# Node ID 10b1e61357ac546e70e590cc79889c0889e7bc22
# Parent  ca417c9464d378661e9e962be1447647006297f3
encoding: teach unitolocal() and unifromlocal() to handle None

These are aliased to str{to,from}local(), along with pycompat.identity(),
depending on the platform.  It's inconsistent that None works on py2 because of
identity(), but not on py3.  This will be needed in the next patch.
Yuya Nishihara - Sept. 23, 2018, 6:49 a.m.
On Sun, 23 Sep 2018 01:18:52 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1537667864 14400
> #      Sat Sep 22 21:57:44 2018 -0400
> # Node ID 10b1e61357ac546e70e590cc79889c0889e7bc22
> # Parent  ca417c9464d378661e9e962be1447647006297f3
> encoding: teach unitolocal() and unifromlocal() to handle None
> 
> These are aliased to str{to,from}local(), along with pycompat.identity(),
> depending on the platform.  It's inconsistent that None works on py2 because of
> identity(), but not on py3.  This will be needed in the next patch.
> 
> diff --git a/mercurial/encoding.py b/mercurial/encoding.py
> --- a/mercurial/encoding.py
> +++ b/mercurial/encoding.py
> @@ -202,10 +202,14 @@ def fromlocal(s):
>  
>  def unitolocal(u):
>      """Convert a unicode string to a byte string of local encoding"""
> +    if u is None:
> +        return None
>      return tolocal(u.encode('utf-8'))
>  
>  def unifromlocal(s):
>      """Convert a byte string of local encoding to a unicode string"""
> +    if s is None:
> +        return None
>      return fromlocal(s).decode('utf-8')

I'm not a fan of making low-level functions untyped. Instead, maybe you can
wrap function calls with pycompat.rapply().
Matt Harbison - Sept. 23, 2018, 1:33 p.m.
> On Sep 23, 2018, at 2:49 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Sun, 23 Sep 2018 01:18:52 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1537667864 14400
>> #      Sat Sep 22 21:57:44 2018 -0400
>> # Node ID 10b1e61357ac546e70e590cc79889c0889e7bc22
>> # Parent  ca417c9464d378661e9e962be1447647006297f3
>> encoding: teach unitolocal() and unifromlocal() to handle None
>> 
>> These are aliased to str{to,from}local(), along with pycompat.identity(),
>> depending on the platform.  It's inconsistent that None works on py2 because of
>> identity(), but not on py3.  This will be needed in the next patch.
>> 
>> diff --git a/mercurial/encoding.py b/mercurial/encoding.py
>> --- a/mercurial/encoding.py
>> +++ b/mercurial/encoding.py
>> @@ -202,10 +202,14 @@ def fromlocal(s):
>> 
>> def unitolocal(u):
>>     """Convert a unicode string to a byte string of local encoding"""
>> +    if u is None:
>> +        return None
>>     return tolocal(u.encode('utf-8'))
>> 
>> def unifromlocal(s):
>>     """Convert a byte string of local encoding to a unicode string"""
>> +    if s is None:
>> +        return None
>>     return fromlocal(s).decode('utf-8')
> 
> I'm not a fan of making low-level functions untyped. Instead, maybe you can
> wrap function calls with pycompat.rapply().

IIUC, you’re saying in the next patch, either change the Windows alias of procutil.tonativestr() to a lambda that uses pycompat.rapply(), or more verbosely, use rapply() everywhere that it converted to Unicode?  Can you explain the “untyped low-level functions” part, and why that’s bad?
Yuya Nishihara - Sept. 23, 2018, 2:11 p.m.
On Sun, 23 Sep 2018 09:33:31 -0400, Matt Harbison wrote:
> > On Sep 23, 2018, at 2:49 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> >> On Sun, 23 Sep 2018 01:18:52 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1537667864 14400
> >> #      Sat Sep 22 21:57:44 2018 -0400
> >> # Node ID 10b1e61357ac546e70e590cc79889c0889e7bc22
> >> # Parent  ca417c9464d378661e9e962be1447647006297f3
> >> encoding: teach unitolocal() and unifromlocal() to handle None
> >> 
> >> These are aliased to str{to,from}local(), along with pycompat.identity(),
> >> depending on the platform.  It's inconsistent that None works on py2 because of
> >> identity(), but not on py3.  This will be needed in the next patch.
> >> 
> >> diff --git a/mercurial/encoding.py b/mercurial/encoding.py
> >> --- a/mercurial/encoding.py
> >> +++ b/mercurial/encoding.py
> >> @@ -202,10 +202,14 @@ def fromlocal(s):
> >> 
> >> def unitolocal(u):
> >>     """Convert a unicode string to a byte string of local encoding"""
> >> +    if u is None:
> >> +        return None
> >>     return tolocal(u.encode('utf-8'))
> >> 
> >> def unifromlocal(s):
> >>     """Convert a byte string of local encoding to a unicode string"""
> >> +    if s is None:
> >> +        return None
> >>     return fromlocal(s).decode('utf-8')
> > 
> > I'm not a fan of making low-level functions untyped. Instead, maybe you can
> > wrap function calls with pycompat.rapply().
> 
> IIUC, you’re saying in the next patch, either change the Windows alias
> of procutil.tonativestr() to a lambda that uses pycompat.rapply(), or more
> verbosely, use rapply() everywhere that it converted to Unicode?

The latter, but only where None is allowed. I hope there aren't many. If
there are, say, 10+ instances, we'll probably want a helper function.

> Can you explain the “untyped low-level functions” part, and why that’s bad?

Because doing that will obfuscate the code. Now we can be sure that "s"
must be bytes if there's strfromlocal(s), but if it accepted None, we might
have to think what if "s" were None.

Patch

diff --git a/mercurial/encoding.py b/mercurial/encoding.py
--- a/mercurial/encoding.py
+++ b/mercurial/encoding.py
@@ -202,10 +202,14 @@  def fromlocal(s):
 
 def unitolocal(u):
     """Convert a unicode string to a byte string of local encoding"""
+    if u is None:
+        return None
     return tolocal(u.encode('utf-8'))
 
 def unifromlocal(s):
     """Convert a byte string of local encoding to a unicode string"""
+    if s is None:
+        return None
     return fromlocal(s).decode('utf-8')
 
 def unimethod(bytesfunc):