Black: Question on forcing black to put one parameter per line

[allan-simon]

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]

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]

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]

@zsol , thank you for your response.

[allan-simon]

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]

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]

826 should address this.

[ambv]

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