Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line-based compactness #747

Closed
neiljp opened this issue Mar 13, 2019 · 5 comments
Closed

Line-based compactness #747

neiljp opened this issue Mar 13, 2019 · 5 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@neiljp
Copy link

neiljp commented Mar 13, 2019

TL; DR: This issue is motivated by black's output being generally longer (line-wise), which means it reduces the line-fit in finite editor/viewer windows, particularly with nested data structures or functions with parameters with many parameters where the lines are just that little bit too long.

By my understanding black currently has a multi-step indenting process to wrap lines, depending upon the 'fit':

  • everything on 1-line
  • 1-line for components; grouping characters on lines before and after
  • multi-line for components; grouping characters on lines before and after

I appreciate that one aspect of this design choice is to minimize diffs by keeping the grouping characters ([, (, {, etc) on separate lines (and including commas), however I'm personally concerned that this comes at the cost of readability when formatting leads code to be spread over many more lines and so not fit as easily in editor windows.

Potential solutions:

  • Try having components/parameters 'stick' to the first/last grouping character, aligning elements vertically, and falling back to grouping characters on separate lines if there is insufficient space
  • Fitting multiple components per line, eg. when a function has more than threshold parameters, dividing the same number between lines and reducing to one per line if that fails

Thanks for your consideration and work on this tool!

@ambv
Copy link
Collaborator

ambv commented Mar 14, 2019

With the current adoption of Black it is impossible for us to make any major changes to how formatting is done. In fact, there is a faction of users who try to convince me to make formatting even more vertically sparse than it already is.

In general I tend to agree with the sparse crowd: lines that are too long make it impossible to quickly find recognizable patterns in calls and nested data structures.

@neiljp
Copy link
Author

neiljp commented Mar 16, 2019

My main objection is perhaps cases such as:

print(
    "some long string"
    "a bit more"
)

which I prefer as:

print("some long string a bit shorter"
      "bit of previous string and a bit more")

and similar cases with if lines being broken.

I don't find this as much of an issue with data-structures on the whole, but some become very verbose. A cut-down example I have would be 7 lines for one element, 5 per additional - rising to 12 for one, 8 per additional (with black). That text is certainly readable as-is.

While I appreciate the clarity that black provides, and that minimizing line lengths is useful, my feeling is that minimizing vertical space is also important and that black as it stands fails to sufficiently consider that factor. If line lengths are a factor, then why not the number of lines taken up by a construct? On that basis, if line length has a 'knob' to tune the formatting, then why not potentially something similar for the number of lines?

It is of course your decision how to support the existing and potential users of black. If this is too much to ask for black as it stands, would you ever consider it for a 'black v2'?

@jimmywan
Copy link

FWIW, completely disagree with OPs point of view and prefer black's way of doing things. It matches the way I've been writing code for 10+ years and generally creates cleaner diffs. Matches up with IntelliJ's Chop Down If Long, rule.

@neiljp
Copy link
Author

neiljp commented Apr 23, 2019

I appreciate the feedback and that this is unlikely to make it into black given the few responses so far, though I don't really feel that my point about tuning for vertical limitations in the same way as horizontal limitations has been considered.

@JelleZijlstra JelleZijlstra added the T: style What do we want Blackened code to look like? label May 5, 2019
@ambv
Copy link
Collaborator

ambv commented Mar 4, 2020

I don't see anything actionable here. Neil's preference is different but in the end it's a subjective manner.

@ambv ambv closed this as completed Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

4 participants