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

Add "preview" and "skip-magic-trailing-comma" options, and version bump to v0.2.5 #48

Merged
merged 2 commits into from
Feb 25, 2022
Merged

Conversation

ferdynice
Copy link
Contributor

Add the "preview" option.

@peterjc
Copy link
Owner

peterjc commented Feb 22, 2022

Do you use this then?

  --preview                       Enable potentially disruptive style changes
                                  that will be added to Black's main
                                  functionality in the next major release.

@ferdynice
Copy link
Contributor Author

Correct. There are a few other options that seem to be missing (I think). Not sure if there were existing plans to add them, but adding at least this specific option would be very helpful for us.

@peterjc
Copy link
Owner

peterjc commented Feb 22, 2022

Yes, as I recall black started with no options (except the line length - notes on that in the flake8-black change log), but has sadly been getting more and more.

I think now that they have dropped the beta tag, it is worthwhile looking at the black Python API for a neater way to use their own configuration parser, and just specify which files to check.

@peterjc
Copy link
Owner

peterjc commented Feb 22, 2022

You're welcome to take a look at that approach - although waiting on psf/black#779 might be wiser.

@ferdynice
Copy link
Contributor Author

I don't think the options will change that often anymore, since the aim of Black is to be as opiniated as possible. Especially now that it is becoming more and more stable. Using a config parser from Black (official API or not) would be nice, but a bit out of scope for me to pick up.

Instead what I did now is add an additional option in this PR, "skip_magic_trailing_comma", which seems to be a highly debated issue. When you set this option to true, magic commas will be turned off. Without magic commas, Black won't look at existing formatting and will try to put arguments/collections as much one 1 line as possible. Check psf/black#1288

I believe with this current set of flags most users will be provided for. But feel free to decide however you see fit.

@ferdynice ferdynice changed the title Add "preview" option Add "preview" and "skip_magic_trailing_comma" options Feb 23, 2022
@peterjc
Copy link
Owner

peterjc commented Feb 23, 2022

That is pragmatic, and I'm OK with merging this with the hope of switching to the planned black API in future which will (hopefully) let us drop this part of the flake8-black code.

Can you do a version bump and change log entry for this too? That would save me some time making the release with these changes. Thanks!

@ferdynice ferdynice changed the title Add "preview" and "skip_magic_trailing_comma" options Add "preview" and "skip-magic-trailing-comma" options Feb 23, 2022
@ferdynice
Copy link
Contributor Author

Done! I hope I did this right.

@ferdynice
Copy link
Contributor Author

I renamed the option in the messages to "skip-magic-trailing-comma", since that is what the actual option in the config file is named. And I tested and verified that it actually works (just like "preview").

@ferdynice ferdynice changed the title Add "preview" and "skip-magic-trailing-comma" options Add "preview" and "skip-magic-trailing-comma" options, and version bump to v0.2.5 Feb 23, 2022
@peterjc peterjc merged commit 938c821 into peterjc:master Feb 25, 2022
@peterjc
Copy link
Owner

peterjc commented Feb 25, 2022

Do you happen to already have a couple of short examples where these settings come into play?

@ferdynice
Copy link
Contributor Author

ferdynice commented Feb 25, 2022

If you mean a short explanation, here you go (assume a line-length = 79 for the first example):

With preview = false (default):

def foo():
    print(
        "hellooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo world"
    )
    print("hello " "world")

With preview = true:

def foo():
    print(
        "hellooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo"
        " world"
    )
    print("hello world")

As you can see it will concatenate and split strings to accomodate Black's styling.

With skip-magic-trailing-comma = false (default):

def foo():
    SomeClass(
        foo=1,
        bar=2,  # remove this trailing comma to instruct black to put all args on a single line
    )

With skip-magic-trailing-comma = true:

def foo():
    SomeClass(foo=1, bar=2)

So without the magic trailing comma (skip-magic-trailing-comma = true), if it fits on a single line, Black will format it that way. No matter if the last arg contained a trailing comma or not.

peterjc added a commit that referenced this pull request Feb 25, 2022
Functionality added in pull request #48
@peterjc
Copy link
Owner

peterjc commented Feb 25, 2022

Lovely - I made something similar with commas, but reminding myself how I setup the test folders took me longer than I expected. I'll use your preview example now...

peterjc added a commit that referenced this pull request Feb 25, 2022
Functionality added in pull request #48, test case
suggested by @ferdynice
@peterjc
Copy link
Owner

peterjc commented Feb 25, 2022

Curious - adding # noqa: E501 to the very long line in your example seems to disable the black string editing. Not to worry, the second example of implicit string concatenation works.

@ferdynice
Copy link
Contributor Author

Ah indeed. Any comment starting with # noqa it seems. But for the purpose to check if the preview option has any effect at all, the concatenation test should suffice.

@peterjc
Copy link
Owner

peterjc commented Feb 25, 2022

That's up on PyPI now https://pypi.org/project/flake8-black/0.2.5/ - conda-forge to follow.

@peterjc
Copy link
Owner

peterjc commented Feb 25, 2022

I forgot this would need the latest black, see #49. Fixed as v0.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants