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

Suboptimal line split choices with wrap_long_dict_values_in_parens #3452

Open
tolomea opened this issue Dec 19, 2022 · 6 comments
Open

Suboptimal line split choices with wrap_long_dict_values_in_parens #3452

tolomea opened this issue Dec 19, 2022 · 6 comments
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@tolomea
Copy link

tolomea commented Dec 19, 2022

Describe the style change

I'm looking at differences between 22.8.0 and 23.1a1 on our code base, most of it looks really good ❤️ as always thanks for the great work

however there is one pattern of change that doesn't seem ideal, it's possible this has been flagged already, I wasn't sure what to look for.

22.8.0

            common.models.DateTimeField: (
                datetime(2020, 1, 31, tzinfo=utc) + timedelta(days=i)
            ),

23.1a1

            common.models.DateTimeField: datetime(2020, 1, 31, tzinfo=utc) + timedelta(
                days=i
            ),

when a line needs breaking there must be some logic to decide where to break it, I guess it will find multiple candidates and rank them somehow, it looks like in this case it's started favouring the later break, whereas before it was favouring the structurally more significant one

@tolomea tolomea added the T: style What do we want Blackened code to look like? label Dec 19, 2022
@tolomea
Copy link
Author

tolomea commented Dec 19, 2022

I can confirm this doesn't happen in 22.12.0, so it's new in the 23 line

@yilei
Copy link
Contributor

yilei commented Dec 19, 2022

This is caused by #3440, agree it made this particular case worse. I can take a look once I have some free time.

@JelleZijlstra
Copy link
Collaborator

Are you running with --preview? #3440 isn't slated for the 2023 stable style, so it's less urgent to fix.

Thanks @tolomea for reporting and @yilei for diagnosing.

@JelleZijlstra JelleZijlstra added F: linebreak How should we split up lines? C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. labels Dec 19, 2022
@tolomea
Copy link
Author

tolomea commented Dec 19, 2022

re: --preview, yes we are, sorry I forgot to mention that, we like black so much we always want more

@ichard26
Copy link
Collaborator

Just for future reference, please make it very clear which style your feedback is for. As you would imagine we care a lot more about feedback for the 2023 stable style and we'll assume any feedback on 23.1a1 to be about it unless noted.

I'll also make this more clear in the main thread and my blog post to avoid confusion :)

@hauntsaninja hauntsaninja changed the title Suboptimal line split choices in 23.1a1 Suboptimal line split choices with wrap_long_dict_values_in_parens Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. 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