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

Boris FELD boris.feld at octobus.net
Wed Feb 13 04:33:15 EST 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