peterjc / flake8-black

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

Design change - prioritise black's configuration #6

Closed peterjc closed 5 years ago

peterjc commented 5 years ago

See discussion on #4 and #5 with @ADKosm, in particular: https://github.com/peterjc/flake8-black/pull/5#issuecomment-497852319

Historically the plugin has taken the flake8 max-line-length setting, and passed this to black as the line-length setting. Other black settings were not considered, and would be defaults (target python version, quotes, etc). Using max-line-length 88 was recommended in the flake8 configuration for using the plugin.

The new plan is to decouple the flake8 max-line-length setting for (E501 validation) from how black is involved. This means if flake8 and black both use 88 (or 120), and black has been run recently, you would get no BLK100 errors and few if any E501 errors.

Is say few, because having used black from some months, it does fail to break long strings - in particular I see this with longer debugging messages, e.g.

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)

after black 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 sometimes useful to set the flake8 max-line-length to be greater than the black line-length, if you have a lot of cases like this.

Under this new design, it would be nice but not essential to have a warning if the flake8 max-line-length is less than the black line-length.

The plugin documentation would continue to recommend setting the flake8 max-line-length to the black line-length, and suggest sticking with black's default of 88.

peterjc commented 5 years ago

Implementation wise, we can start from the work in #4 by @ADKosm (which is already on the master branch), but perhaps there is a better entry point for calling black on a single file, and letting it load the configuration on its own.

We could at an extreme do a shell call to run black --check "path/to/file.py" but would have performance overheads. As per the plugin's documentation, that is the desired effect here.

Given the change in behaviour, this change will deserve at least a minor version jump from 0.0.x to 0.1.0

peterjc commented 5 years ago

TL;DR - looking at line length as real world configurations on GitHub, the plan for flake8-black v0.1.0 to use the black configuration will be fine or an improvement.

This github search works fairly well to find pyproject.toml files using the tool.black settings:

https://github.com/search?l=TOML&p=2&q=%22pyproject.toml%22+%22tool.black%22&type=Code

Lots of cases setting different line lengths, 120 is popular.

Found one example where black is told to use 79 (via pyproject.toml) to match the flake8 default (settings in tox.ini):

https://github.com/darvid/pitstop/blob/master/pyproject.toml https://github.com/darvid/pitstop/blob/master/tox.ini

Another example where both black and flake8 told to us 80:

https://github.com/kevinoid/python-project-template/blob/master/pyproject.toml https://github.com/kevinoid/python-project-template/blob/master/setup.cfg

or:

https://github.com/Unbabel/OpenKiwi/blob/0.1.1/pyproject.toml https://github.com/Unbabel/OpenKiwi/blob/0.1.1/setup.cfg

Where both black and flake8 told to us 88:

https://github.com/mikevansighem/thea/blob/v0.0.1/pyproject.toml https://github.com/mikevansighem/thea/blob/v0.0.1/tox.ini

Where both told to use 120 (there are more from the same author):

https://github.com/pawamoy/ansito/blob/v0.1.0/.flake8 https://github.com/pawamoy/ansito/blob/v0.1.0/pyproject.toml

From this sample the changes planned for flake8-black v0.1.0 to ignore the flake8 max line length setting, and use the black line-length setting will be absolutely fine.

Here's a more interesting example where black uses 88, while the flake8 max line length is 100 - this would have messed up with flake8-black v0.0.x but will be fine with the plan for v0.1.0:

https://github.com/Woile/commitizen/blob/v1.5.0/pyproject.toml https://github.com/Woile/commitizen/blob/v1.5.0/.flake8

peterjc commented 5 years ago

Quote examples are rarer - this search only found three https://github.com/search?l=TOML&q=%22pyproject.toml%22+%22tool.black%22+%22skip-string-normalization%22&type=Code

https://github.com/darvid/pitstop/blob/master/pyproject.toml https://github.com/kevinoid/python-project-template/blob/master/pyproject.toml https://github.com/Unbabel/OpenKiwi/blob/0.1.1/pyproject.toml

Excludes does get used, mostly for folders, but sometimes even for single files e.g.

https://github.com/gwtwod/py3humiocli/blob/bb78dd383f98540d3fdfb4dcb954d555ee00e2a0/pyproject.toml

Perhaps we should get flake8-black to obey the black exclude settings too - but the same functionality is possible with the configuration in recent versions of flake8.

peterjc commented 5 years ago

Attempted to clarify excluding files in c43bef64c64247ff08f9b07a657848b4bf961233, this is now out as v0.1.0