On Wed, Feb 13, 2019 at 10:33:15AM +0100, Boris FELD wrote: > On 01/12/2018 02:35, Matt Harbison wrote: > > On Fri, 30 Nov 2018 07:25:04 -0500, Boris FELD > > <boris.f...@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:
If you're talking to Łukasz, could you float the way clang-format works by him? That is, if a trailing comma is present in the literal it's formatted one element per line, but if there's no trailing comma it's formatted compactly. I've found that to be a good tradeoff. It'd also make our import blocks format correctly without modification. > > For example: > > requestdata = json.dumps({ > # > 'objects': objects, > 'operation': action, > > }) I...guess I can live with that. Begrudgingly. > > 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@mercurial-scm.org > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel