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

Respect comments on different lines #1353

Closed
NakulK48 opened this issue Apr 22, 2020 · 7 comments
Closed

Respect comments on different lines #1353

NakulK48 opened this issue Apr 22, 2020 · 7 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

@NakulK48
Copy link

Describe the style change A clear and concise description of how the style can be
improved.

If a line has a comment at the end of it, it shouldn't be combined with adjacent lines

Examples in the current Black style Think of some short code snippets that show
how the current Black style is not great:

(n.b. run with --line-length 120)

parametrize(
    [5, [1],]  # output should not be primitive  # output list must have same length as input
)

As you can see, it's much less obvious which comment refers to which entry.

Desired style How do you think Black should format the above snippets:

parametrize([
    5,  # output should not be primitive
    [1],  # output list must have same length as input
])

(unchanged from how I wrote it)

Additional context Add any other context about the problem here.

This came up when I was trying to parametrize a test and leave some comments explaining each of the cases.

@NakulK48 NakulK48 added the T: style What do we want Blackened code to look like? label Apr 22, 2020
@JelleZijlstra JelleZijlstra added the F: comments The syntactic kind. Not in the language grammar, always on our minds. Best bugs. label May 30, 2021
@goerz
Copy link

goerz commented Jun 22, 2021

Just another example of where this happens, with function arguments:

def generate_unsplit_data(
    rf,
    T=5e5,  # μs
    nt=5001,
    z_max=50,  # μm
    mass=86,  # Dalton
):
    pass

gets reformatted as

def generate_unsplit_data(
    rf, T=5e5, nt=5001, z_max=50, mass=86,  # μs  # μm  # Dalton
):
    pass

I would expect Black to leave the original code unchanged.

@JelleZijlstra
Copy link
Collaborator

@goerz that code is left unchanged on the latest release (due to the magic trailing comma).

@goerz
Copy link

goerz commented Jun 22, 2021

Can confirm. I was still running 20.8b1. After updating to 21.6b0, the code remains unchanged if there's a trailing comma. 🎉 That solves my particular problem, but I'd still argue that even without the trailing comma, Black probably shouldn't reformat the snippet. Comments should prevent any combination with surrounding lines, as stated in the OP.

To be fair, though, the new trailing-coma-behavior probably makes this a much less pressing issue. As far as I can tell, it also applies to the OP, and I can't really think of an example where one couldn't add a trailing comma to get around Black's behavior 🤷

@randolf-scholz
Copy link

This is still an issue when using constructs without magic comma. For example, I like to do the following:

__all__ = (
    [  # Classes
        "Class1",
        "Class2",
        "Class3",
    ] + [  # Functions
        "function1",
        "function2",
        "function3",
    ]
)

which black (version 21.9b0) will reformat to

__all__ = ["Class1", "Class2", "Class3",] + [  # Classes  # Functions
    "function1",
    "function2",
    "function3",
]

UGLY!

@NakulK48
Copy link
Author

NakulK48 commented Sep 28, 2021

__all__ = (
    [  # Classes
        "Class1",
        "Class2",
        "Class3",
    ] + [  # Functions
        "function1",
        "function2",
        "function3",
    ]
)

Couldn't you just do:

__all__ = [  
    # Classes
    "Class1",
    "Class2",
    "Class3",
    # Functions
    "function1",
    "function2",
    "function3",
]

@randolf-scholz
Copy link

@NakulK48 Yes, I realize my example was not that good and it should probably be rewritten in the way you presented. There might be other examples that are more applicable where some terms are literal and others are dynamic. Nevertheless I still think even if the way I originally wrote it is sub-optimal it is still way better than what black converted it to.

@JelleZijlstra
Copy link
Collaborator

This is more cases of #379.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2023
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

4 participants