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

Black 19.10b0 no longer fixes short argument lists to a single line #1136

Closed
jdufresne opened this issue Nov 4, 2019 · 8 comments
Closed
Labels
F: trailing comma Full of magic R: not a bug This is deliberate behavior of Black.

Comments

@jdufresne
Copy link
Contributor

Describe the bug

In previous versions of Black, short argument lists would be wrapped to a single line. In new versions of Black, this is not always the case.

If the argument list starts off as a single line, it'll remain a single line. If the argument list starts as multiple lines, it'll remain multiple lines. See test case below.

To Reproduce

test.py

my_func(
    "a",
    "b",
)

With Black 19.3b0

@@ -1,5 +1,2 @@
-my_func(
-    "a",
-    "b",
-)
+my_func("a", "b")

With Black 19.10b0

@@ -1,5 +1,4 @@
 my_func(
-    "a",
-    "b",
+    "a", "b",
 )

Expected behavior

I expect the behavior experienced in 19.3b0.

Environment

  • Version: 19.10b0 (also master)
  • OS and Python version: Fedora 30 Python 3.7.5

Does this bug also happen on master?

Yes.

https://black.now.sh/?version=master&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ABjAEZdAD2IimZxl1N_WmBpTZzyXdYgGAumRPJ9fzc8wrFrsw5q41oEoZU7ytbygjdQqX-b0UdsSVEi1fbjjwWIr_X0wjnhZyweKwAAAABPNX1pNP1yrQABYmRPGZI9H7bzfQEAAAAABFla

@jdufresne jdufresne added the T: bug Something isn't working label Nov 4, 2019
@jdufresne
Copy link
Contributor Author

@jdufresne
Copy link
Contributor Author

Both bisected to commit 9854d4b.

CC @durin42

@jdufresne
Copy link
Contributor Author

Perhaps another version of #1129. They bisect to the same commit.

@durin42
Copy link
Contributor

durin42 commented Nov 5, 2019

This is WAI: as part of #826 we have to preserve trailing commas. It's a little surprising to me that it reformats that argument list /at all/, but I do recall something is "funny" about function calls from when I wrote the patch.

@jdufresne
Copy link
Contributor Author

FWIW, as an end user, I find this new beahvior quite suprising and a move away from the opinionated philosophy I really love about Black. When I write Python code, I almost never think about formatting anymore because I know Black will take of it later. With this change, it means I'm back to thinking about formatting. "Should I add a trailing comma or remove it?", is now a question I'll be asking before submitting code.

I normally blindly add a trailing comma to all code. Not because I want to keep arguments across multiple lines, but just out of habit. I don't think about it much more than that and let Black figure it out later.

The PR that introduce this change is all about preserving multiple lines in collections. So what does that have to do with argument lists of function calls? Are you intentionally grouping argument lists in with collections here, or is that an unintended side effect of the implementation?

I think there is another problem with this change. Consider a very long argument list:

my_func("abcabcabc123123", "abcabcabc123123", "abcabcabc123123", "abcabcabc123123", "abcabcabc123123")

After formatting using master, this becomes (notice a trailing comma was added):

my_func(
    "abcabcabc123123",
    "abcabcabc123123",
    "abcabcabc123123",
    "abcabcabc123123",
    "abcabcabc123123",
)

Now -- perhaps due to refactoring -- the last 2 arguments are removed and reformatted:

my_func(
    "abcabcabc123123", "abcabcabc123123", "abcabcabc123123",
)

Now, because of this new change and the added trailing comma, Black doesn't reformat the line back to a single line.

@vemel
Copy link
Contributor

vemel commented Nov 19, 2019

Trailing comma should be handled differently for LHS and RHS. For LHS it can be safely removed from a function definition. For RHS it should always be preserved.

@vemel
Copy link
Contributor

vemel commented Nov 20, 2019

I created a new logic for RHS and LHS values, please test my PR. Also, please let me know if output changes work for you, as it changed dramatically in some cases.

Also, give me some ideas when RHS-value should be rendered as a single line and LHS shuld be splitted. THings some to my mind:

  • RHS has # type: ignore and LHS does not
  • LHS is longer that RHS? - probably, but could lead to unstable output easily.

@JelleZijlstra
Copy link
Collaborator

This is intended behavior of the magic trailing comma. As of recent releases, you can use --skip-magic-trailing-comma if you don't want this behavior.

@ichard26 ichard26 added F: trailing comma Full of magic R: not a bug This is deliberate behavior of Black. and removed T: bug Something isn't working labels May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: trailing comma Full of magic R: not a bug This is deliberate behavior of Black.
Projects
None yet
Development

No branches or pull requests

5 participants