peterjc / flake8-black

flake8 plugin to run black for checking Python coding style
MIT License
166 stars 10 forks source link

v0.1.0 with pyproject.toml support #5

Closed peterjc closed 5 years ago

peterjc commented 5 years ago

Do these follow on changes from #4 make sense @ADKosm?

Mostly cosmetic - documentation and rewording and renaming the new code to BLK800.

There is a functional change, stopping the length conflict being triggered if there was no pyproject.toml file present, and the user had something other than 88 as the max line length via flake8.

What do you think should happen if the flake8 configuration gives a line length (e.g. 120, something different to the black default) and while there is a pyproject.toml file present it does not specify the length? Right now this assumes the presence of any black configuration file means 88, and so gives the error.

I aim to release this early next week (doing a release late on a Friday is a bad plan).

ADKosm commented 5 years ago

@peterjc If there is no pyproject.toml or pyproject.toml without line-length paramerter, it doesn't means, that black will prefer any line length. In this case, black will use default value for line-length = 88 (see https://github.com/python/black). Therefore, line-length for black will always defined and we can check accordance with flake line-length in any case, even if there is no additional configuration for black.

peterjc commented 5 years ago

Your comments, and my own growing experience using black regularly, have made me reconsider the way this plugin has treated line length.

In practise, I have been using the black default 88, and telling flake8 to use that too - and given that is the documented recommendation, I would expect most people are doing that too.

I have however noticed that sometimes black will leave very long lines in place - it does not break long strings, for example:

def some_function(some_args):
    """"Just an example."""
    # some code
    sys.stderr.write("A very long string as a single variable in Python does not get broken by black, even if it exceeds the desired line length which could be %i or whatever" % 88)

becomes:

def some_function(some_args):
    """"Just an example."""
    # some code
    sys.stderr.write(
        "A very long string as a single variable in Python does not get broken by black, even if it exceeds the desired line length which could be %i or whatever"
        % 88
    )

So, it is actually useful to set the flake8 max line length to be equal or greater than the black value E501 validation.

I now think the flake8-plugin should run black with whatever settings black would use itself (including the Python version targeted, and the quotes, etc) - regardless of the flake8 max line length.

It would be nice to give a warning if the flake8 max line length is set lower than the black line length.

ADKosm commented 5 years ago

I like idea of allowing line-lenght for flake to be equal or grater than black line-length!

Do I understand correctly that you are planning to change the predicate for checking conflicts and do this check on every launch (even if there is no pyproject.toml ?

peterjc commented 5 years ago

At this point I am thinking if we decouple the length settings, we don't need the conflict warning. The user will simply get lots of E501 messages on files where black has left some long lines, or conversely lots of BLK100 messages on files where black would like to use longer lines.

I don't currently know how to do the configuration conflict check at startup, but that would seem better. Note that it is possible on a large code base for there to be more than one `pyproject.toml so perhaps the conflict error per file is acceptable?

peterjc commented 5 years ago

As part of this planned change, if there is no pyproject.toml file, I think we should start calling black with its default settings (not whatever flake8's max-line-length is).

peterjc commented 5 years ago

OK, I have pulled some of these changes directly to master, and then rebased.

I am going to close this in favour of the new approach now described in new issue #6.