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

Question on forcing black to put one parameter per line #737

Closed
allan-simon opened this issue Mar 7, 2019 · 8 comments
Closed

Question on forcing black to put one parameter per line #737

allan-simon opened this issue Mar 7, 2019 · 8 comments
Labels
F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. T: style What do we want Blackened code to look like?

Comments

@allan-simon
Copy link
Contributor

allan-simon commented Mar 7, 2019

Operating system:
Python version: Linux
Black version: 18.9b0
Does also happen on master: master was not working at the moment this ticken was written :/

Currently we want to format our code as this

foo(
   arg1=val1,
   arg2=val2,
)

as we agree to black arguments as exposed in your readme:

https://github.com/ambv/black#how-black-wraps-lines

. Such formatting produces smaller diffs; when you add or remove an element, it's always just one line.

however black force to reformat this as such

foo(
  arg1=val1, arg2=val2, arg3=val3
)

that become inconsistent in the following case

# get indented "correctly"
foo(
   arg1=val1_with_long_name,
   arg2=val2_with_long_name,
)
# get reindent in one line
foo(
   arg1=val1_with_long_name,
   arg2=val2,
)

the second one get reindented in one line, with arg2 at the end, making it very easy to miss it while scanning with the eye
moreover it means that replacing val2 by a little longer string, will suddenly cause unwanted reindentation (which can create a snowball effect in a big diff that rename a variable into one with a longer name)

To summarize the main problem is that black is quite strict, but it does not let you be stricter than him :/

@rahulrajaram
Copy link

rahulrajaram commented Mar 22, 2019

Yes, I am actually surprised that this isn't something black does considering it resulting diff is cleaner. I'm curious to known what the maintainers of black think.

@zsol
Copy link
Collaborator

zsol commented Mar 22, 2019

I understand where you're coming from and this is somewhat related to the discussion in #747. If you're asking for my personal opinion, I prefer what black currently does because

  1. Black's current output (if it fits on a line, it belongs on a line) makes it easier for me to process more code than if it'd always explode arguments, because the code is more compact but not unreadably so.
  2. I find that the diff size increase this behavior causes is tolerable because one side of the diff is going to be a single line, so it can't be that much code. Here's an example: black collapsed three lines into one because a string literal got shortened so the whole argument list can now fit on the line. I don't think that change is very hard to read.

@rahulrajaram
Copy link

@zsol , thank you for your response.

@allan-simon
Copy link
Contributor Author

for 2. it really depends how much is changed , if you're refactoring an attribute name used accross your code-base it can significantly increase the diff

for 1 i beg to disagree that having to manually parse yourself where are the , which is not the most obvious character is tiring , especially as you have no obvious rule on when it will be split. On the other hand being sure that you're sure that 3 arguments => 3 lines, make it very easy to scan , less error prone, especially when code-reviewing other's code.

@allan-simon
Copy link
Contributor Author

I just got one case where I think black is plain wrong
consider the following code

INTERNAL_IPS = (
    "127.0.0.1",
    "172.17.0.1",  # for docker
)

black insists on converting it to

INTERNAL_IPS = ("127.0.0.1", "172.17.0.1")  # for docker

here black prevents me from documenting correctly code :(

@JelleZijlstra JelleZijlstra added F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. T: style What do we want Blackened code to look like? labels May 5, 2019
@JelleZijlstra
Copy link
Collaborator

#826 should address this.

@zsol zsol added style and removed style labels Jul 22, 2019
@ambv
Copy link
Collaborator

ambv commented Mar 4, 2020

This is partially addressed already but needs refinement. We'll address the rest of the concerns here with #1288.

@ambv ambv closed this as completed Mar 4, 2020
@iQiexie
Copy link

iQiexie commented Sep 3, 2022

https://stackoverflow.com/a/66155767/13246657

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

6 participants