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

Create a --preview flag for disruptive changes between major updates #2751

Closed
felix-hilden opened this issue Jan 7, 2022 · 9 comments · Fixed by #2752
Closed

Create a --preview flag for disruptive changes between major updates #2751

felix-hilden opened this issue Jan 7, 2022 · 9 comments · Fixed by #2752
Labels
C: configuration CLI and configuration T: documentation Improvements to the docs (e.g. new topic, correction, etc) T: enhancement New feature or request

Comments

@felix-hilden
Copy link
Collaborator

As discussed in #2394, we'd like to shove disruptive style changes between major updates behind a --preview flag. And I think it should be created and available even if such changes have not been implemented yet, so that our CLI is consistent.

Perhaps:

  • add the CLI option
  • create a separate style docs section for describing the style
  • from now on announce these kinds of changes under a "preview" header in our changelog
  • document a consistent way of writing preview tests without disrupting stable (I think just duplicating might be fine)

Thoughts, suggestions?


@ichard26 shall we add this to the "stable" project?

@felix-hilden felix-hilden added T: enhancement New feature or request C: configuration CLI and configuration T: documentation Improvements to the docs (e.g. new topic, correction, etc) labels Jan 7, 2022
@felix-hilden
Copy link
Collaborator Author

felix-hilden commented Jan 10, 2022

How should we separate the different features though? Surely the check for "preview" could be made in multiple places even for a single feature. So come time to make it stable, it could be difficult to find all the places where that particular feature is used.

We could perhaps employ a behind-the-scenes feature flag (or list or whatever) system. Maybe the simplest one would be just to use a set of all preview features, which is filled or emptied depending on the flag state.

class Preview(Enum):
    esp = auto()
    style = auto()
    disruption = auto()

preview = set(list(Preview))

def visit_String(self):
    if Preview.esp in preview:
        do_magic

This would of course work equivalently and faster by just using a boolean, but it might be worth it to set up a more readable system.

@JelleZijlstra
Copy link
Collaborator

Yes, that sounds like a good approach. We could also just use the existing Mode object, and when we create that we set whatever flags we want to True based on the value of the --preview flag.

@felix-hilden
Copy link
Collaborator Author

Yeah that works too! Any preferences by others?

@felix-hilden
Copy link
Collaborator Author

Although, if we're creating a public API and if Mode is a part of it, maybe we should simplify its creation. Or rather, disallow setting individual features 😄

@ichard26
Copy link
Collaborator

Sigh, we do have to be careful with that black.Mode has been part of Black's informal API since forever unfortunately but yeah I'd be in favour of simplifying its interface to only what's also exposed at the CLI level. Sounds like it'd introduce churn but honestly if you're configuring syntax detection black.Features I don't consider it reasonable to support your usecase.

@felix-hilden
Copy link
Collaborator Author

I didn't quite get your last point, sorry. What would introduce churn? But yeah, in addition to the informal API argument, I feel like Mode could be a good candidate for the eventually defined formal public API, since it pretty much reflects the CLI.

@felix-hilden
Copy link
Collaborator Author

@JelleZijlstra your thoughts on Mode? Then I'll update the PR so we can get to ESP next 👍

@JelleZijlstra
Copy link
Collaborator

JelleZijlstra commented Jan 14, 2022

One approach could be that we keep Mode append-only on the assumption that it will be part of the API, but add a separate internal object that stores all feature gates independently.

For example, maybe in a few months --preview will include ESP, pow hugging, and parenthesized a = (b == c). Then we'd have something like:

@dataclass
class DetailedMode:
    experimental_string_processing: bool = False
    pow_hugging: bool = False
    parenthesized_assignments: bool = False
    @classmethod
    def from_mode(cls, mode: Mode) -> Self:
        return cls(
            experimental_string_processing=mode.experimental_string_processing or mode.preview,
            pow_hugging=mode.preview,
            parenthesized_assignments=mode.preview,
        )

We would pass this object around instead of Mode within the formatting code.

@felix-hilden
Copy link
Collaborator Author

Yeah that could work! Or we could even have it as a (private?) attribute of Mode. I'll have to test things.

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: documentation Improvements to the docs (e.g. new topic, correction, etc) T: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants