-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Enable enable-unstable-feature = ['string_processing'] without preview flag #4196
Comments
Thanks for the issue! This was intentional. Our goal with With that said, I'm sympathetic to the fact that despite being explicitly experimental |
IMHO Best way forward would be to make experimental string processing non experimental :D We have been using it for past year (on around 500k LOC) and it work without a problem (compatible with ruff, flake8, pylint). Without having (only) experimental processing latest black has became a bit useless for us :( Also on larger code base it would be benefical to enable just some parts of preview features (in case they can be applied in isolation). It would be easier to test and not introduce too many changes in the code. The preview functionality gets in the end cherry picked anyway making it even better argument for such isolated preview features. Why not allow us to do the same during the development for next year style and be more engaged in the future black style :) |
Could you provide more detail on this? It sounds like a bug and I don't think that's Black's normal behavior.
This is why we added the new
If you want this to happen, there's a long list of issues related to it listed in #4042 that need to be fixed. |
Sure. So our configration is like this:
And then you have such code that is correctly formatted via experimental string processing (actually logger.info but I replaced it with print).
Now when you disable it ... (preview and enable-unstable-feature). You get:
And second line exceeds 120 chars making ruff, flake8 and other checkers complain.
And we have tons of logging.info() with long messages that are getting corrupted by this issue (for 2022-2024 black). All the issues are related to string.format ... we use them also raise exceptions.SomeError(".. very long string {id} ... ".format(id=id, ) |
@JelleZijlstra did you manage to reproduce the issue that we are facing with current black offical string processing with format? I just remembered that I posted this issue a year ago and it was just closed as that you don't want to change the style during the year and experimental processing will likely be stable in 2024: For now we are sticking with 2023 version because 2024 in current state is unfournately broken. |
I can confirm what you're seeing, but it only works if the line length matches up exactly:
I could see us make a change where we move Realistically I don't see us turning on string-processing by default. I opened #4208 about this. |
Is that preview feature require whole preview enabled making it basically very hard to test and still not usable in our case. Having preview style enabled one by one would be super useful. For solving the bug under #4208 is more than just solving this concrete bug. I don't mind the proposed changes but I can see combining strings that are splitted in code would make less readable choice for some string being stuck under contraversal umbrealla for another year or two. But still testing the impact in isolation coming back to first comment and this bug as a whole. |
That is not true. The current |
Sorry for missunderstanding but I saw 50% more changes in preview than going to 2024 (our numbers bellow). Nevertheless preview is still a moving target even if a bit more stable (than unstable). Every black we are in fear what will be broken again and feature set will stabilize only near end of the year. Just to put the things into perspective (our perspective in huge monolit):
Findings:
The crash:
|
Basically we have 5 options:
Timing is on 2019 macbook pro with 32gb. Findings:
Since all the options are getting equally painful to maintain for us, I think we will eventually go in ruff direction and their own interpreation of black style. But for now we stay with 2023 black. I will monitor #4208 in the meantime. |
Your timings are likely distorted by cache effects and by the fact that Black is a lot slower when it changes files (because it does more checks). |
Hm ... oh yes I see ... changing files takes a huge amount of time in black. 2024 preview with zero files changed is much better and does not regress between versions:
Sorry for false alarm ... |
Describe the bug
In 2023 we used experimental-string-processing. This was enabled without preview style enabled.
However now in 2024 the experimental string processing flag was removed and replaced with
enable-unstable-feature = ['string_processing']
However this feature requires turning on the preview feature.
And now the files are additionally formated based on the preview style.
To Reproduce
vs:
We would like to go according to official style but have better string processing enabled. Without that we get bunch of line too long errors in the code.
For example, take this code (first without preview and the second with):
Expected behavior
I like that you are formalizing the enable-unstable-feature in such way but not that it requires preview with additional modifications. So unstable processing without other preview style (like in 2023).
Environment
The text was updated successfully, but these errors were encountered: