Patchwork [8,of,9] pycompat: put single line things above class and function definitions

login
register
mail settings
Submitter Pulkit Goyal
Date June 15, 2017, 9:34 p.m.
Message ID <c95d3227e37cfbbb2c68.1497562490@workspace>
Download mbox | patch
Permalink /patch/21411/
State Accepted
Headers show

Comments

Pulkit Goyal - June 15, 2017, 9:34 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1497561930 -19800
#      Fri Jun 16 02:55:30 2017 +0530
# Node ID c95d3227e37cfbbb2c687dad98bc978d063c624f
# Parent  66940f7bf570ebb3a3a43c556e6887dab28c19a4
pycompat: put single line things above class and function definitions

Earlier, there are some single line assignments initially then a function, then
some more single line assignments, then again few functions. This patch gathers
all those single line assignments at one place and make sure functions are
continuous so that it's easy to read code. Therefore it also moves the wrapper
above the bytestr class.
Yuya Nishihara - June 16, 2017, 2:21 p.m.
On Fri, 16 Jun 2017 03:04:50 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1497561930 -19800
> #      Fri Jun 16 02:55:30 2017 +0530
> # Node ID c95d3227e37cfbbb2c687dad98bc978d063c624f
> # Parent  66940f7bf570ebb3a3a43c556e6887dab28c19a4
> pycompat: put single line things above class and function definitions
> 
> Earlier, there are some single line assignments initially then a function, then
> some more single line assignments, then again few functions. This patch gathers
> all those single line assignments at one place and make sure functions are
> continuous so that it's easy to read code. Therefore it also moves the wrapper
> above the bytestr class.

Maybe we've been using different algorithms to sort stuffs in pycompat.py.
I tried to keep py2 and py3 functions in the same order. I'm okay for this
change, but can you adjust the sorting methodology a bit so that related
functions won't be orphaned?

> diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
> --- a/mercurial/pycompat.py
> +++ b/mercurial/pycompat.py
> @@ -78,6 +78,20 @@
>  
>      bytechr = struct.Struct('>B').pack
>  
> +    def _wrapattrfunc(f):
> +        @functools.wraps(f)
> +        def w(object, name, *args):
> +            return f(object, sysstr(name), *args)
> +        return w
> +
> +    # these wrappers are automagically imported by hgloader
> +    delattr = _wrapattrfunc(builtins.delattr)
> +    getattr = _wrapattrfunc(builtins.getattr)
> +    hasattr = _wrapattrfunc(builtins.hasattr)
> +    setattr = _wrapattrfunc(builtins.setattr)
> +    xrange = builtins.range
> +    unicode = str

I want to place bytestr next to bytechr.

> +
>      class bytestr(bytes):
>          """A bytes which mostly acts as a Python 2 str

> -    strkwargs = identity
> -    byteskwargs = identity
> -
> -    oslinesep = os.linesep
> -    osname = os.name
> -    ospathsep = os.pathsep
> -    ossep = os.sep
> -    osaltsep = os.altsep
> -    stdin = sys.stdin
> -    stdout = sys.stdout
> -    stderr = sys.stderr
>      if getattr(sys, 'argv', None) is not None:
>          sysargv = sys.argv
> -    sysplatform = sys.platform
> -    getcwd = os.getcwd
> -    sysexecutable = sys.executable
> -    shlexsplit = shlex.split
> -    stringio = cStringIO.StringIO
> -    maplist = map

Leaving only sysargv doesn't make much sense to me.
Pulkit Goyal - June 17, 2017, 8:52 a.m.
On Fri, Jun 16, 2017 at 7:51 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 16 Jun 2017 03:04:50 +0530, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1497561930 -19800
>> #      Fri Jun 16 02:55:30 2017 +0530
>> # Node ID c95d3227e37cfbbb2c687dad98bc978d063c624f
>> # Parent  66940f7bf570ebb3a3a43c556e6887dab28c19a4
>> pycompat: put single line things above class and function definitions
>>
>> Earlier, there are some single line assignments initially then a function, then
>> some more single line assignments, then again few functions. This patch gathers
>> all those single line assignments at one place and make sure functions are
>> continuous so that it's easy to read code. Therefore it also moves the wrapper
>> above the bytestr class.
>
> Maybe we've been using different algorithms to sort stuffs in pycompat.py.
> I tried to keep py2 and py3 functions in the same order. I'm okay for this
> change, but can you adjust the sorting methodology a bit so that related
> functions won't be orphaned?

I was just using linear scan to look for functions and hence decided
to adjust them as given in the patch. But I like your idea of keeping
them in same order, so let's keep this as it is.

Patch

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -78,6 +78,20 @@ 
 
     bytechr = struct.Struct('>B').pack
 
+    def _wrapattrfunc(f):
+        @functools.wraps(f)
+        def w(object, name, *args):
+            return f(object, sysstr(name), *args)
+        return w
+
+    # these wrappers are automagically imported by hgloader
+    delattr = _wrapattrfunc(builtins.delattr)
+    getattr = _wrapattrfunc(builtins.getattr)
+    hasattr = _wrapattrfunc(builtins.hasattr)
+    setattr = _wrapattrfunc(builtins.setattr)
+    xrange = builtins.range
+    unicode = str
+
     class bytestr(bytes):
         """A bytes which mostly acts as a Python 2 str
 
@@ -192,20 +206,6 @@ 
             return doc
         return sysbytes(doc)
 
-    def _wrapattrfunc(f):
-        @functools.wraps(f)
-        def w(object, name, *args):
-            return f(object, sysstr(name), *args)
-        return w
-
-    # these wrappers are automagically imported by hgloader
-    delattr = _wrapattrfunc(builtins.delattr)
-    getattr = _wrapattrfunc(builtins.getattr)
-    hasattr = _wrapattrfunc(builtins.hasattr)
-    setattr = _wrapattrfunc(builtins.setattr)
-    xrange = builtins.range
-    unicode = str
-
     def open(name, mode='r', buffering=-1):
         return builtins.open(name, sysstr(mode), buffering)
 
@@ -262,6 +262,25 @@ 
     sysstr = identity
     strurl = identity
     bytesurl = identity
+    # In Python 2, fsdecode() has a very chance to receive bytes. So it's
+    # better not to touch Python 2 part as it's already working fine.
+    fsdecode = identity
+    strkwargs = identity
+    byteskwargs = identity
+    oslinesep = os.linesep
+    osname = os.name
+    ospathsep = os.pathsep
+    ossep = os.sep
+    osaltsep = os.altsep
+    stdin = sys.stdin
+    stdout = sys.stdout
+    stderr = sys.stderr
+    sysplatform = sys.platform
+    getcwd = os.getcwd
+    sysexecutable = sys.executable
+    shlexsplit = shlex.split
+    stringio = cStringIO.StringIO
+    maplist = map
 
     # this can't be parsed on Python 3
     exec('def raisewithtb(exc, tb):\n'
@@ -279,35 +298,14 @@ 
             raise TypeError(
                 "expect str, not %s" % type(filename).__name__)
 
-    # In Python 2, fsdecode() has a very chance to receive bytes. So it's
-    # better not to touch Python 2 part as it's already working fine.
-    fsdecode = identity
-
     def getdoc(obj):
         return getattr(obj, '__doc__', None)
 
     def getoptb(args, shortlist, namelist):
         return getopt.getopt(args, shortlist, namelist)
 
-    strkwargs = identity
-    byteskwargs = identity
-
-    oslinesep = os.linesep
-    osname = os.name
-    ospathsep = os.pathsep
-    ossep = os.sep
-    osaltsep = os.altsep
-    stdin = sys.stdin
-    stdout = sys.stdout
-    stderr = sys.stderr
     if getattr(sys, 'argv', None) is not None:
         sysargv = sys.argv
-    sysplatform = sys.platform
-    getcwd = os.getcwd
-    sysexecutable = sys.executable
-    shlexsplit = shlex.split
-    stringio = cStringIO.StringIO
-    maplist = map
 
 empty = _queue.Empty
 queue = _queue.Queue