Auto-formatting code with black - object now if you have a strong opinion

Matt Harbison mharbison72 at gmail.com
Fri Nov 30 20:35:08 EST 2018


On Fri, 30 Nov 2018 07:25:04 -0500, Boris FELD <boris.feld at octobus.net>  
wrote:

> I think using automatic formatting is a great idea and we should move
> forward with this plan. Black seems a good option. I share other's
> concerns about the formatting of import. I also wonder if this also
> applies to list and dict formatting that we tend to express with one
> value per line for clarity.

It looks like yes, unfortunately, if it fits on one line:

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -289,50 +289,47 @@ class _gitlfsremote(object):
          Return decoded JSON object like {'objects': [{'oid': '', 'size':  
1}]}
          See  
https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md
          """
-        objects = [{'oid': p.oid(), 'size': p.size()} for p in pointers]
-        requestdata = json.dumps({
-            'objects': objects,
-            'operation': action,
-        })
-        url = '%s/objects/batch' % self.baseurl
+        objects = [{"oid": p.oid(), "size": p.size()} for p in pointers]
+        requestdata = json.dumps({"objects": objects, "operation":  
action})
+        url = "%s/objects/batch" % self.baseurl
          batchreq = util.urlreq.request(url, data=requestdata)
...


I'm also concerned about stuff like this, which seems far less readable  
than the original (especially the conditional):

diff --git a/hgext/relink.py b/hgext/relink.py
--- a/hgext/relink.py
+++ b/hgext/relink.py
@@ -56,29 +50,32 @@ def relink(ui, repo, origin=None, **opts
      command is running. (Both repositories will be locked against
      writes.)
      """
-    if (not util.safehasattr(util, 'samefile') or
-        not util.safehasattr(util, 'samedevice')):
-        raise error.Abort(_('hardlinks are not supported on this system'))
-    src = hg.repository(repo.baseui, ui.expandpath(origin or  
'default-relink',
-                                          origin or 'default'))
-    ui.status(_('relinking %s to %s\n') % (src.store.path,  
repo.store.path))
+    if not util.safehasattr(util, "samefile") or not util.safehasattr(
+        util, "samedevice"
+    ):
+        raise error.Abort(_("hardlinks are not supported on this system"))
+    src = hg.repository(
+        repo.baseui, ui.expandpath(origin or "default-relink", origin or  
"default")
+    )
+    ui.status(_("relinking %s to %s\n") % (src.store.path,  
repo.store.path))
      if repo.root == src.root:

This was actually in the first file that I randomly sampled.  I think  
there were a few more instances like this, but don't feel motivated to  
find them now.  There were a bunch of lines (in lfs anyway) that were  
flattened out, and were more readable.  But that was before I saw that the  
default formatting is 88 columns.  So maybe allowing longer lines would  
help?  (At the cost of possibly rolling up more lists and dicts into a  
single line.)

I'm not adamantly opposed, and the idea of combining version control and  
tooling to enforce a format is intriguing.  But FWIW, I'm not thrilled  
with the result of this.


More information about the Mercurial-devel mailing list