john-hen / Flake8-pyproject

Flake8 plug-in loading the configuration from pyproject.toml
https://pypi.org/project/Flake8-pyproject
MIT License
218 stars 10 forks source link

Override Flake8's aggregate_options #20

Closed mattbwilmes closed 1 year ago

mattbwilmes commented 1 year ago

This PR addresses the concern brought up in https://github.com/john-hen/Flake8-pyproject/issues/19

The issue was addressed by overriding Flake8's aggregate_options() function, similar to how Flake8's parse_config() is being overwritten currently.

This solution was seen as preferable to simply calling parse_known_args() because it allows this plug-in to work with calls to flake8.main.cli.main() (i.e. it doesn't assume that --toml_config is coming from the command line)

Testing

Wrote the following simple script to make sure it worked

import flake8.main.cli

flake8.main.cli.main(["--toml-config=repo/pyproject.toml", "test_file.py"])

Also ran the following command from my home directory to make sure it still worked

flake8 --toml-config=repo/pyproject.toml test_file.py

Confirmed that python3 tools/test.py still passes all tests

john-hen commented 1 year ago

Thanks for the PR!

As you can see, the test on Windows failed. That's because you assume there's an environment variable called HOME, which is not a given.

I also noticed that path names with a ~ in them are not properly interpreted and an error will occur claiming the file doesn't exist. This was resolved by replacing the ~ with os.getenv('HOME')

You'll have to remove that entirely. This is not supposed to work, not when doing this:

import flake8.main.cli

flake8.main.cli.main(["--toml-config=~/repo/pyproject.toml", "test_file.py"])

The expansion of the tilde is a feature of the Unix shell, which is not involved when you pass the arguments directly. It would work if you ran that command in the terminal. But replicating shell features is really not "our concern".

mattbwilmes commented 1 year ago

The expansion of the tilde is a feature of the Unix shell, which is not involved when you pass the arguments directly. It would work if you ran that command in the terminal. But replicating shell features is really not "our concern".

Yikes, that was a really bad assumption on my part! I'm so used to working with code that is meant to run on Ubuntu that this hadn't crossed my mind... I'm glad there are tests in place to flag this type of stuff :100:

I pushed a change to remove this: https://github.com/john-hen/Flake8-pyproject/pull/20/commits/f0a9e5da845dd130e26a25f897522840b6afa765 And realized I forgot to remove import os (fixed in https://github.com/john-hen/Flake8-pyproject/pull/20/commits/34c658a7635d7595a02630e43b2f423dd0101dd5)