-
Notifications
You must be signed in to change notification settings - Fork 288
Flake8 plugin for linting #656
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
Conversation
The version is specified due to the hard way the code uses flake8 that might not work if a code change was introduced in flake8 codebase
|
Test is failing because of the latest release of pylint #661 |
|
Looks good to me! |
|
@gatesn As flake8_config is now aggregating flake8 config for both pycodestyle and mccabe, we should update it to load the config under plugins.falke8, right? Users will no longer be able to use flake8 config files with pycodestyle and mccabe that way, the flake8 plugin will be the only one to use flake8 config files. |
setup.py
Outdated
| extras_require={ | ||
| 'all': [ | ||
| 'autopep8', | ||
| 'flake8==3.7.8', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though we use internal APIs, let’s not pin this. Otherwise we’ll end up conflicting with people’s own Flake8 versions.
We should just track latest the best we can I reckon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
|
|
||
| @hookimpl | ||
| def pyls_lint(config, document): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we default this plugin to disabled, and when it is enabled disable pep8 and pyflakes? I think at the moment we’d end up with duplicate diagnostics which could be annoying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, they shouldn't be running at the same time, and disabling flake8 by default is a good idea so current users don't feel the change but can enable linting directly via flake8 if they want to. But flake8_config have to be made loading config into flake8 plugin as well from now on.
As per the discussion here #190 a way of supporting flake8 linting is by implementing an additional plugin that will use the flake8 config directly instead of passing it to other linting tools like pycodestyle and pyflakes (even if flake8 is a wrapper around those tools by itself).
Flake8 lacks (for now) of a stable public python API which should be implemented in the future, so I figured out a dirty way from the source code to access the report results and get the information we need to report back to the IDE, however, this should be updated whenever a python API is made available.