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

Split preview into --preview and --unstable? #4093

Closed
JelleZijlstra opened this issue Dec 10, 2023 · 3 comments · Fixed by #4096
Closed

Split preview into --preview and --unstable? #4093

JelleZijlstra opened this issue Dec 10, 2023 · 3 comments · Fixed by #4096
Labels
C: configuration CLI and configuration T: enhancement New feature or request

Comments

@JelleZijlstra
Copy link
Collaborator

Black's --preview mode is supposed to give a preview of what next year's stable style will look like. But for the last few years, it hasn't been able to do that job, because --preview includes experimental string processing, which produces numerous bugs and likely again won't make it into the 2024 stable style (#4042). Several other current preview features likely won't make it into the 2024 stable style.

I propose to add a new style, --unstable. The idea would be that --preview is only for new style features that we currently think would make it into next year's stable style, and --unstable is for new features that we would like to eventually stabilize, but that are currently still buggy. --unstable would include all --preview features. This way, --preview will truly be a preview.

Concrete rules:

  • Any preview feature that leads to new crashes (e.g., stability bugs) should be demoted to --unstable until the issue is fixed.
  • Similarly, if a preview feature is shown to lead to bad formatting in some cases (in our judgment as maintainers), we demote the feature to unstable. For example, Black unexpectedly alters comments and one-liner expressions when comments are placed above, when using --preview #3555 should make us demote the related preview feature to unstable.
  • New preview features merged in November or December would by default go into --unstable, so that there is enough time for the ecosystem to try out the new feature. We can make exceptions for sufficiently simple and uncontroversial changes. We'd move these features into --preview once the new year starts.

Alternative solutions considered:

  • We could simply be more aggressive in deleting preview features that cause trouble. I'd personally be hesitant to do this though, especially for features that have had significant effort invested into them: it will be a lot more effort to apply them to the Black codebase again later once the original issue is fixed. However, I do think we should delete unstable features that have remained in that state for too long.
  • We could move unstable features out of --preview, but not add a separate --unstable flag. Instead, we could add a private flag like -X string_processing to enable individual preview features. This would allow more targeted testing, but it opens the door to more configurability than I'd like, and might lead to users expecting us to continue supporting such fine-grained configuration control.
@JelleZijlstra JelleZijlstra added T: enhancement New feature or request C: configuration CLI and configuration labels Dec 10, 2023
@amyreese
Copy link
Contributor

I'd love the "rules" to go a step further and follow something like this:

  • new features go to --unstable by default
  • move features into --preview once they have proven stable and/or had positive enough feedback that the features are all but guaranteed to be in the next year's stable release

Ie, more of a "features land to preview when they're widely agreed upon and proven, and later land to stable on the next year's release".

Whether or not that is a meaningful semantic difference for black's development/release cycle, I'm not sure, but I like the idea that --unstable provides a better place for monitoring and providing feedback, while --preview can be reviewed and validated more closely, knowing that style changes there have been vetted more thoroughly.

For large codebases like Meta's, I would also love the ability to selectively enable individual preview features when running black via its API, so that we could evaluate each one separate from the context of other style changes, but without necessarily exposing CLI flags that people will expect to rely on.

As is, running with --preview or the 24.1a1 alpha release makes it very difficult to know what formatting changes are caused by which features in #4042, and even more difficult when some features have been rejected but are still "active" in these preview modes.

@JelleZijlstra
Copy link
Collaborator Author

Thanks! I think the amount of feedback we get on the preview style is low enough that it doesn't make sense to move features into the lower "unstable" tier by default, so I'd prefer to keep moving new features into "preview" first. I do try to be quite careful before merging even a preview feature into main.

For large codebases like Meta's, I would also love the ability to selectively enable individual preview features when running black via its API

You can probably hack this together by subclassing black.Mode, for what it's worth.

As is, running with --preview or the 24.1a1 alpha release makes it very difficult to know what formatting changes are caused by which features in #4042, and even more difficult when some features have been rejected but are still "active" in these preview modes.

What's in 24.1a1 should match what is expected to be in the new stable style (though, of course, still subject to change).

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 25, 2024

I think something like this is important and I would have loved for it to be around in 2023.

However, one big problem is if we move features from --preview to --unstable (and then hopefully soon back to --preview) that this can cause a lot of churn for users who want to use preview (in cases where formatting isn't preserved, like hug parens). I'm fine with running into a bug when using --preview. I'm fine with some code changes. But if we have big thrashy yo-yo swings I wouldn't use it for anything real. As maintainers, we'll be incentivised to be cautious about removing things from preview style and we further winnow the set of people who even know that they could be providing us feedback.

The only real solution I see here is to bite the bullet and add --enable-unstable-hidden-feature X. This way, if hug parens was good enough on your preview-style codebase and we relegated it to unstable hidden, you have a way to upgrade Black without thrash. You would then only eventually incur thrash in the event that we deleted a preview->hidden feature entirely, which we've yet to do.

Some semantics:

  • Most features are in --preview; the --enable-unstable-hidden-feature flag only matters for unstable hidden features
  • In particular, it is impossible to selectively disable / enable features in --preview
  • We provide no compatibility guarantees on the existence of hidden features or their behaviour

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: configuration CLI and configuration T: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants