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

black flake8 incompatibility #2348

Closed
morenoh149 opened this issue Dec 4, 2019 · 5 comments
Closed

black flake8 incompatibility #2348

morenoh149 opened this issue Dec 4, 2019 · 5 comments

Comments

@morenoh149
Copy link
Contributor

What happened?

I am using the precommit hooks as instructed at https://cookiecutter-django.readthedocs.io/en/latest/developing-locally.html

I have also chosen black as my formatter in the wizard as well as in my editor. When I go to commit the following form


class FooForm(forms.ModelForm):
    objective = forms.CharField(
        required=False, widget=forms.Textarea(attrs={"placeholder": "Objective",}),
    )

    class Meta:
        model = Foo
        fields = [
            "objective",
        ]

I get the error

$ gc
Trim Trailing Whitespace.............................(no files to check)Skipped
flake8...................................................................Failed
hookid: flake8

foo/forms.py:10:80: E231 missing whitespace after ','

Thats for the CharField trailing comma in the keyword args.

What should've happened instead?

Flake8 should not complain about black's formatting.

Steps to reproduce

[//]: make a cookiecutter app and select black
[//]: install precommit hooks
[//]: make a form like mine and attempt to commit

Solution

Should we add https://github.com/peterjc/flake8-black ?

@luzfcb
Copy link
Collaborator

luzfcb commented Dec 4, 2019

@morenoh149 Thanks again to report another problem. Did you use flake8-black before? I haven't had time to try it yet... If you find a fix for this, feel free to submit a pull-request.

@morenoh149
Copy link
Contributor Author

morenoh149 commented Dec 4, 2019

I have not used that plugin even though it was the first thing to show up for the incompatibility. For now I just removed the precommit with rm .git/hooks/pre-commit (or mv .git/hooks/pre-commit .git/hooks/pre-commit.bkup) 😁

@demestav
Copy link
Contributor

demestav commented Feb 6, 2020

@morenoh149 please correct me if I am wrong, but the trailing comma in the dictionary you specify does not serve a purpose, so it should be removed, which will pass flake8 check. In my understanding, you may want to add a trailing comma in a single (or more) element dictionaries, to minimize diffs when inserting new lines. But for that to be true, the dictionary should be written like

attrs = {
    "placeholder": "Objective",
}

Regarding tools to help in this situation, I use flake8-commas. Checking against your example, it does raise an error on that trailing comma. @luzfcb Maybe we can include this in requirements? I did not thoroughly checked if it is fully compatible with black though, but that seems the case. (EDIT2: Just found this: psf/black#551).

EDIT: By the way, here is a relevant issue on Black repository psf/black#274

@browniebroke
Copy link
Member

Also see psf/black#1288

@browniebroke
Copy link
Member

I'm going to close this, as I don't think there is much we can do on our end, it's an issue that should solved soon in Black.

Thanks for the report!

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

No branches or pull requests

4 participants