Patchwork D5539: style: run yapf on a subset of mercurial

login
register
mail settings
Submitter phabricator
Date Jan. 9, 2019, 8:29 p.m.
Message ID <differential-rev-PHID-DREV-46us5zxjqujotcr7nbud-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/37591/
State New
Headers show

Comments

phabricator - Jan. 9, 2019, 8:29 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I've tried to pick knobs that more or less conform to our current
  style. I ran the formatter on the same set of files as the black
  proposal (see https://phab.mercurial-scm.org/D5064), but it resulted in many fewer edits.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5539

AFFECTED FILES
  .style.yapf
  mercurial/dirstateguard.py
  mercurial/httpconnection.py
  mercurial/lsprofcalltree.py
  mercurial/mergeutil.py
  mercurial/minifileset.py
  mercurial/node.py
  mercurial/policy.py
  mercurial/pushkey.py
  mercurial/rcutil.py
  mercurial/scmposix.py
  mercurial/scmwindows.py
  mercurial/state.py
  tests/test-check-code.t

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 9, 2019, 8:36 p.m.
durin42 added a comment.


  I see a few minor issues here, but it's at least plausible?

INLINE COMMENTS

> minifileset.py:57-58
>          symbols = {
> -            'all': lambda n, s: True,
> -            'none': lambda n, s: False,
> -            'size': lambda n, s: _sizep(tree[2])(s),
> +            'all': lambda n,
> +            s: True,
> +            'none': lambda n,

yapf bug here, filed as https://github.com/google/yapf/issues/662

> scmwindows.py:53
>      home = os.path.expanduser('~')
> -    path = [os.path.join(home, 'mercurial.ini'),
> -            os.path.join(home, '.hgrc')]
> +    path = [os.path.join(home, 'mercurial.ini'), os.path.join(home, '.hgrc')]
>      userprofile = encoding.environ.get('USERPROFILE')

We could force this to one-per-line by adding a trailing comma.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5539

To: durin42, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -42,6 +42,7 @@ 
   .hgsigs
   .hgtags
   .jshintrc
+  .style.yapf
   CONTRIBUTING
   CONTRIBUTORS
   COPYING
diff --git a/mercurial/state.py b/mercurial/state.py
--- a/mercurial/state.py
+++ b/mercurial/state.py
@@ -4,7 +4,6 @@ 
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
-
 """
 This file contains class to wrap the state for commands and other
 related logic.
diff --git a/mercurial/scmwindows.py b/mercurial/scmwindows.py
--- a/mercurial/scmwindows.py
+++ b/mercurial/scmwindows.py
@@ -32,7 +32,8 @@ 
             if f.endswith('.rc'):
                 rcpath.append(os.path.join(progrcd, f))
     # else look for a system rcpath in the registry
-    value = util.lookupreg('SOFTWARE\\Mercurial', None,
+    value = util.lookupreg('SOFTWARE\\Mercurial',
+                           None,
                            winreg.HKEY_LOCAL_MACHINE)
     if not isinstance(value, str) or not value:
         return rcpath
@@ -49,8 +50,7 @@ 
 def userrcpath():
     '''return os-specific hgrc search path to the user dir'''
     home = os.path.expanduser('~')
-    path = [os.path.join(home, 'mercurial.ini'),
-            os.path.join(home, '.hgrc')]
+    path = [os.path.join(home, 'mercurial.ini'), os.path.join(home, '.hgrc')]
     userprofile = encoding.environ.get('USERPROFILE')
     if userprofile and userprofile != home:
         path.append(os.path.join(userprofile, 'mercurial.ini'))
diff --git a/mercurial/scmposix.py b/mercurial/scmposix.py
--- a/mercurial/scmposix.py
+++ b/mercurial/scmposix.py
@@ -22,9 +22,11 @@ 
     rcs = [os.path.join(path, 'hgrc')]
     rcdir = os.path.join(path, 'hgrc.d')
     try:
-        rcs.extend([os.path.join(rcdir, f)
-                    for f, kind in util.listdir(rcdir)
-                    if f.endswith(".rc")])
+        rcs.extend([
+            os.path.join(rcdir,
+                         f) for f,
+            kind in util.listdir(rcdir) if f.endswith(".rc")
+        ])
     except OSError:
         pass
     return rcs
@@ -53,13 +55,17 @@ 
         if confighome is None or not os.path.isabs(confighome):
             confighome = os.path.expanduser('~/.config')
 
-        return [os.path.expanduser('~/.hgrc'),
-                os.path.join(confighome, 'hg', 'hgrc')]
+        return [
+            os.path.expanduser('~/.hgrc'),
+            os.path.join(confighome,
+                         'hg',
+                         'hgrc')
+        ]
 
 def termsize(ui):
     try:
         import termios
-        TIOCGWINSZ = termios.TIOCGWINSZ  # unavailable on IRIX (issue3449)
+        TIOCGWINSZ = termios.TIOCGWINSZ # unavailable on IRIX (issue3449)
     except (AttributeError, ImportError):
         return 80, 24
 
diff --git a/mercurial/rcutil.py b/mercurial/rcutil.py
--- a/mercurial/rcutil.py
+++ b/mercurial/rcutil.py
@@ -43,9 +43,15 @@ 
     if env is None:
         env = encoding.environ
     checklist = [
-        ('EDITOR', 'ui', 'editor'),
-        ('VISUAL', 'ui', 'editor'),
-        ('PAGER', 'pager', 'pager'),
+        ('EDITOR',
+         'ui',
+         'editor'),
+        ('VISUAL',
+         'ui',
+         'editor'),
+        ('PAGER',
+         'pager',
+         'pager'),
     ]
     result = []
     for envname, section, configname in checklist:
diff --git a/mercurial/pushkey.py b/mercurial/pushkey.py
--- a/mercurial/pushkey.py
+++ b/mercurial/pushkey.py
@@ -22,11 +22,16 @@ 
         n.pop('obsolete')
     return n
 
-_namespaces = {"namespaces": (lambda *x: False, _nslist),
-               "bookmarks": (bookmarks.pushbookmark, bookmarks.listbookmarks),
-               "phases": (phases.pushphase, phases.listphases),
-               "obsolete": (obsolete.pushmarker, obsolete.listmarkers),
-              }
+_namespaces = {
+    "namespaces": (lambda *x: False,
+                   _nslist),
+    "bookmarks": (bookmarks.pushbookmark,
+                  bookmarks.listbookmarks),
+    "phases": (phases.pushphase,
+               phases.listphases),
+    "obsolete": (obsolete.pushmarker,
+                 obsolete.listmarkers),
+}
 
 def register(namespace, pushkey, listkeys):
     _namespaces[namespace] = (pushkey, listkeys)
diff --git a/mercurial/policy.py b/mercurial/policy.py
--- a/mercurial/policy.py
+++ b/mercurial/policy.py
@@ -24,11 +24,16 @@ 
 policy = b'allow'
 _packageprefs = {
     # policy: (versioned package, pure package)
-    b'c': (r'cext', None),
-    b'allow': (r'cext', r'pure'),
-    b'cffi': (r'cffi', None),
-    b'cffi-allow': (r'cffi', r'pure'),
-    b'py': (None, r'pure'),
+    b'c': (r'cext',
+           None),
+    b'allow': (r'cext',
+               r'pure'),
+    b'cffi': (r'cffi',
+              None),
+    b'cffi-allow': (r'cffi',
+                    r'pure'),
+    b'py': (None,
+            r'pure'),
 }
 
 try:
@@ -65,28 +70,43 @@ 
 
 # keep in sync with "version" in C modules
 _cextversions = {
-    (r'cext', r'base85'): 1,
-    (r'cext', r'bdiff'): 3,
-    (r'cext', r'mpatch'): 1,
-    (r'cext', r'osutil'): 4,
-    (r'cext', r'parsers'): 12,
+    (r'cext',
+     r'base85'): 1,
+    (r'cext',
+     r'bdiff'): 3,
+    (r'cext',
+     r'mpatch'): 1,
+    (r'cext',
+     r'osutil'): 4,
+    (r'cext',
+     r'parsers'): 12,
 }
 
 # map import request to other package or module
 _modredirects = {
-    (r'cext', r'charencode'): (r'cext', r'parsers'),
-    (r'cffi', r'base85'): (r'pure', r'base85'),
-    (r'cffi', r'charencode'): (r'pure', r'charencode'),
-    (r'cffi', r'parsers'): (r'pure', r'parsers'),
+    (r'cext',
+     r'charencode'): (r'cext',
+                      r'parsers'),
+    (r'cffi',
+     r'base85'): (r'pure',
+                  r'base85'),
+    (r'cffi',
+     r'charencode'): (r'pure',
+                      r'charencode'),
+    (r'cffi',
+     r'parsers'): (r'pure',
+                   r'parsers'),
 }
 
 def _checkmod(pkgname, modname, mod):
     expected = _cextversions.get((pkgname, modname))
     actual = getattr(mod, r'version', None)
     if actual != expected:
         raise ImportError(r'cannot import module %s.%s '
-                          r'(expected version: %d, actual: %r)'
-                          % (pkgname, modname, expected, actual))
+                          r'(expected version: %d, actual: %r)' % (pkgname,
+                                                                   modname,
+                                                                   expected,
+                                                                   actual))
 
 def importmod(modname):
     """Import module according to policy and check API version"""
diff --git a/mercurial/node.py b/mercurial/node.py
--- a/mercurial/node.py
+++ b/mercurial/node.py
@@ -11,6 +11,7 @@ 
 
 # This ugly style has a noticeable effect in manifest parsing
 hex = binascii.hexlify
+
 # Adapt to Python 3 API changes. If this ends up showing up in
 # profiles, we can use this version only on Python 3, and forward
 # binascii.unhexlify like we used to on Python 2.
diff --git a/mercurial/minifileset.py b/mercurial/minifileset.py
--- a/mercurial/minifileset.py
+++ b/mercurial/minifileset.py
@@ -40,8 +40,9 @@ 
             f = lambda n, s: n.startswith(p) and (len(n) == pl
                                                   or n[pl:pl + 1] == '/')
             return f
-        raise error.ParseError(_("unsupported file pattern: %s") % name,
-                               hint=_('paths must be prefixed with "path:"'))
+        raise error.ParseError(
+            _("unsupported file pattern: %s") % name,
+            hint=_('paths must be prefixed with "path:"'))
     elif op in {'or', 'patterns'}:
         funcs = [_compile(x) for x in tree[1:]]
         return lambda n, s: any(f(n, s) for f in funcs)
@@ -53,23 +54,27 @@ 
         return lambda n, s: not _compile(tree[1])(n, s)
     elif op == 'func':
         symbols = {
-            'all': lambda n, s: True,
-            'none': lambda n, s: False,
-            'size': lambda n, s: _sizep(tree[2])(s),
+            'all': lambda n,
+            s: True,
+            'none': lambda n,
+            s: False,
+            'size': lambda n,
+            s: _sizep(tree[2])(s),
         }
 
         name = filesetlang.getsymbol(tree[1])
         if name in symbols:
             return symbols[name]
 
         raise error.UnknownIdentifier(name, symbols.keys())
-    elif op == 'minus':     # equivalent to 'x and not y'
+    elif op == 'minus': # equivalent to 'x and not y'
         func1 = _compile(tree[1])
         func2 = _compile(tree[2])
         return lambda n, s: func1(n, s) and not func2(n, s)
     elif op == 'list':
-        raise error.ParseError(_("can't use a list in this context"),
-                               hint=_('see \'hg help "filesets.x or y"\''))
+        raise error.ParseError(
+            _("can't use a list in this context"),
+            hint=_('see \'hg help "filesets.x or y"\''))
     raise error.ProgrammingError('illegal tree: %r' % (tree,))
 
 def compile(text):
diff --git a/mercurial/mergeutil.py b/mercurial/mergeutil.py
--- a/mercurial/mergeutil.py
+++ b/mercurial/mergeutil.py
@@ -15,8 +15,10 @@ 
 
 def checkunresolved(ms):
     if list(ms.unresolved()):
-        raise error.Abort(_("unresolved merge conflicts "
-                            "(see 'hg help resolve')"))
+        raise error.Abort(
+            _("unresolved merge conflicts "
+              "(see 'hg help resolve')"))
     if ms.mdstate() != 's' or list(ms.driverresolved()):
-        raise error.Abort(_('driver-resolved merge conflicts'),
-                          hint=_('run "hg resolve --all" to resolve'))
+        raise error.Abort(
+            _('driver-resolved merge conflicts'),
+            hint=_('run "hg resolve --all" to resolve'))
diff --git a/mercurial/lsprofcalltree.py b/mercurial/lsprofcalltree.py
--- a/mercurial/lsprofcalltree.py
+++ b/mercurial/lsprofcalltree.py
@@ -86,8 +86,8 @@ 
             out_file.write(b'calls=%d 0\n' % subentry.callcount)
         else:
             out_file.write(b'cfi=%s\n' % pycompat.sysbytes(code.co_filename))
-            out_file.write(b'calls=%d %d\n' % (
-                subentry.callcount, code.co_firstlineno))
+            out_file.write(b'calls=%d %d\n' % (subentry.callcount,
+                                               code.co_firstlineno))
 
         totaltime = int(subentry.totaltime * 1000)
         out_file.write(b'%d %d\n' % (lineno, totaltime))
diff --git a/mercurial/httpconnection.py b/mercurial/httpconnection.py
--- a/mercurial/httpconnection.py
+++ b/mercurial/httpconnection.py
@@ -43,8 +43,10 @@ 
         # requires authentication. Since we can't know until we try
         # once whether authentication will be required, just lie to
         # the user and maybe the push succeeds suddenly at 50%.
-        self._progress = ui.makeprogress(_('sending'), unit=_('kb'),
-                                         total=(self.length // 1024 * 2))
+        self._progress = ui.makeprogress(
+            _('sending'),
+            unit=_('kb'),
+            total=(self.length // 1024 * 2))
 
     def read(self, *args, **kwargs):
         ret = self._data.read(*args, **kwargs)
diff --git a/mercurial/dirstateguard.py b/mercurial/dirstateguard.py
--- a/mercurial/dirstateguard.py
+++ b/mercurial/dirstateguard.py
@@ -34,8 +34,8 @@ 
         self._active = False
         self._closed = False
         self._backupname = 'dirstate.backup.%s.%d' % (name, id(self))
-        self._narrowspecbackupname = ('narrowspec.backup.%s.%d' %
-                                      (name, id(self)))
+        self._narrowspecbackupname = ('narrowspec.backup.%s.%d' % (name,
+                                                                   id(self)))
         repo.dirstate.savebackup(repo.currenttransaction(), self._backupname)
         narrowspec.savebackup(repo, self._narrowspecbackupname)
         self._active = True
@@ -50,26 +50,26 @@ 
 
     def close(self):
         if not self._active: # already inactivated
-            msg = (_("can't close already inactivated backup: %s")
-                   % self._backupname)
+            msg = (_("can't close already inactivated backup: %s") %
+                   self._backupname)
             raise error.Abort(msg)
 
         self._repo.dirstate.clearbackup(self._repo.currenttransaction(),
-                                         self._backupname)
+                                        self._backupname)
         narrowspec.clearbackup(self._repo, self._narrowspecbackupname)
         self._active = False
         self._closed = True
 
     def _abort(self):
         narrowspec.restorebackup(self._repo, self._narrowspecbackupname)
         self._repo.dirstate.restorebackup(self._repo.currenttransaction(),
-                                           self._backupname)
+                                          self._backupname)
         self._active = False
 
     def release(self):
         if not self._closed:
             if not self._active: # already inactivated
-                msg = (_("can't release already inactivated backup: %s")
-                       % self._backupname)
+                msg = (_("can't release already inactivated backup: %s") %
+                       self._backupname)
                 raise error.Abort(msg)
             self._abort()
diff --git a/.style.yapf b/.style.yapf
new file mode 100644
--- /dev/null
+++ b/.style.yapf
@@ -0,0 +1,8 @@ 
+[style]
+based_on_style = pep8
+blank_lines_around_top_level_definition = 1
+spaces_before_comment = 1
+column_limit = 80
+split_all_comma_separated_values = true
+space_between_ending_comma_and_closing_bracket = false
+allow_multiline_lambdas = true