-
Notifications
You must be signed in to change notification settings - Fork 10
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 prototype plugin #8
Conversation
Hi, thanks a lot for this PR! So it seems to be possible. As you can see, the tests don't pass. In the Github Actions workflow, it cannot install the package as is (for some reason). Though when I ran the tests locally, skipping that installation step, there were also errors, such as this one:
|
I've found fixes for the build issue, including a syntax problem because I forgot which function I was using...and I did figure out how to run the tests...still fails on the config_flake8 / config_setup / config_tox tests, though, I think because the code now looks at parent directories first for the pyproject.toml... I'll try to dig into it a bit later, but I suspect reverting the "look at parent directory" part would address that one...in which case how do we ensure the tests don't look at the parent? Technically overriding HOME (to the fixture root, for example) should do that, but it doesn't seem to work... I'm probably going to try going back to the start, refactoring the existing hook to ensure all tests still pass, then adding the plugin, then adding the directory-traversal. This may require manually specifying @john-hen Maybe you can document the environment setup, or confirm that I'm doing the right thing for local testing?
|
Hi Victor. So you get the test environment setup when you install with Speaking of running things "in the project root folder"... I decided against implementing this "directory travsersal" that Flake8 does. Most tools, such as pyTest (see above), do not do that. Which is why I closed #7 as "won't fix". It's simpler that way and, I think, adds fairly little. And yes, that's most likely why those tests fail. There is a way to solve this, but we'd have to replicate more of Flake8's internal logic. As we'd have to look for So it's only about getting the hook to run as a plug-in when |
Thanks for that information, John. I removed the tree-traversal part, but kept the refactoring. I also had a slight bug in the entrypoint group name...oops. Added tests to require the plugin, and then confirm the output. It seems to be working properly now in both plugin and |
It works now, thanks a lot. Great job. I will merge this now but will eventually refactor much of it before the next release. This installs a different hook when called as a plug-in, which works similarly to what I outlined in my comment in #5. So now we hook into I see nothing wrong with this approach now that I tested your code. I was unsure before. (And also, didn't get this far to actually make it work.) However, as it stands, we have both versions of the hook in the code base, each doing essentially the same. And when called via |
This is awesome! Thanks ppl! |
It's rather difficult to modify
flake8
configuration before plugins are loaded...at least as part of the standard configuration-loading process, so some monkeypatching was required...Some work seems to be in progress on actually providing a plugin interface for configuration changes (see https://github.com/PyCQA/flake8/blob/main/src/flake8/options/config.py#L88, refactor ~9 months ago), but I couldn't find any GitHub issues tracking the actual addition of a
flake8.configuration
plugin type, so this usesflake8.extension
(with a name ofCFG99
, which hopefully won't have any plugin name collisions).I did test the initial plugin code with a simpler project, using setuptools, and it worked, but then I refactored to unify with what was here and add the plugin.
The updated code also traverses the filesystem to attempt to locate
pyproject.toml
in a parent directory, similar toflake8
, though obviously it would have trouble with nested projects.This PR should resolve #5 via autoconfiguring as a plugin, and should also address #7 due to the refactor / new find-file logic.