Auto-formatting code with black - object now if you have a strong opinion
Boris FELD
boris.feld at octobus.net
Wed Feb 13 09:33:15 UTC 2019
On 01/12/2018 02:35, Matt Harbison wrote:
> 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)
> ...
We have been discussing with the Black author about how we could handle
those cases and we found a `hack` which is adding an empty comment on
the first line of a list, dict, multi-line construction:
For example:
requestdata = json.dumps({
   #
   'objects': objects,
   'operation': action,
})
Would get transformed into:
requestdata = json.dumps(
   {
       #
       "objects": objects,
       "operation": action,
   }
)
which is then stable.
> 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:
Black output is not final yet, Black author wants to have the
possibility to make bugfixes. This particular example might be a bug
that could be solved. It could also be solved by extracting parameters
into variables.
>
> 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.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list