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

Do not split assert conditions on multiple lines #1190

Open
ovidiu-munteanu opened this issue Dec 5, 2019 · 3 comments
Open

Do not split assert conditions on multiple lines #1190

ovidiu-munteanu opened this issue Dec 5, 2019 · 3 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@ovidiu-munteanu
Copy link

Black should not split conditions in assert statements over multiple lines unless necessary. Instead, it should split the assert error message over multiple lines.

Given a long assert statement such as this:

assert type(queryset) in (ModelBase, QuerySet), "You must specify a model or override the class 'queryset'."

Based on the current style black reformats it like this:

assert type(queryset) in (
    ModelBase,
    QuerySet,
), "You must specify a model or override the class 'queryset'."

Instead, I believe it would look much better if it were formatted like this:

assert type(queryset) in (ModelBase, QuerySet), (
    "You must specify a model or override the class 'queryset'."
)
@ovidiu-munteanu ovidiu-munteanu added the T: style What do we want Blackened code to look like? label Dec 5, 2019
@mk-pmb
Copy link

mk-pmb commented Dec 10, 2019

I think this boils down to predicting whether the items in the list will change in the future. Having them eachin one line helps reduce diff flickering, thus helps automatic versioning merges, and also (in some inferior editors) makes it easier to sort the list.

@rpkilby
Copy link

rpkilby commented May 13, 2020

I think this boils down to predicting whether the items in the list will change in the future.

I think that's actually just coincidence. e.g., take this simple assert:

# input
assert False, 'safdlkafs lkasfljfas sadfljkklsadf asdfjl afsdljk sadfl kasdflkjsdflkj df sfd asdf'

# black
assert (
    False
), 'safdlkafs lkasfljfas sadfljkklsadf asdfjl afsdljk sadfl kasdflkjsdflkj df sfd asdf'

Note that black produces the preferred output when the message is sufficiently long:

# input
assert type(queryset) in (ModelBase, QuerySet), 'asdfkj aljkasdfljk df fdjklasdf fdjkafjkl dfakljafs djklasfldk sadfljkaf sdjklfdsa fdajklfadsljk afkjl aflskjjlakfsd asdjl'

# black
assert type(queryset) in (ModelBase, QuerySet), (
    'asdfkj aljkasdfljk df fdjklasdf fdjkafjkl dfakljafs djklasfldk sadfljkaf sdjklfdsa'
    ' fdajklfadsljk afkjl aflskjjlakfsd asdjl'
)

@miriaford
Copy link

Please see #1483 (my duplicate issue). I think the current behavior of Black is just incorrect.

@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label May 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

5 participants