Skip to content
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

Provide configuration option to run flake8 from project root. #2858

Merged
merged 1 commit into from
Aug 31, 2020

Conversation

kalekseev
Copy link
Contributor

Option per-file-ignores was introduced in flake8 version 3.7.0.
It allows to ignore specific errors in specific files using glob syntax.
For example per-file-ignores = src/generated/*.py:F401 will
ignore F401 error in all python files in src/generated.
Thus ale have to run flake8 from project root where .flake8 config
is placed otherwise glob won't match linted file.

Not sure how that should be handled in ale style but I'm ready to follow on this pr.

@stale
Copy link

stale bot commented Aug 13, 2020

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs a bot will close automatically label Aug 13, 2020
@kalekseev
Copy link
Contributor Author

@w0rp please take a look, that feature can be useful

@stale stale bot removed the stale PRs a bot will close automatically label Aug 13, 2020
@Congee
Copy link

Congee commented Aug 21, 2020

I'm surprised this PR has been pending for almost a year over a "considered bug"

@Congee
Copy link

Congee commented Aug 21, 2020

what's blocking this PR?

function! ale_linters#python#flake8#GetCdString(buffer) abort
let l:change_directory = ale#Var(a:buffer, 'python_flake8_change_directory')

if l:change_directory == 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do better than 2. Let's add strings for the existing options and new option, and also continue to support 1 and 0. Make sure to update the documentation, including the documented type.

Let's make this new setting the new default, and make sure we fall back on setting the CWD to the directory the file is in, in the Vader tests.

Option `per-file-ignores` was introduced in flake8 version 3.7.0.
It allows to ignore specific errors in specific files using glob syntax.
For example `per-file-ignores = src/generated/*.py:F401` will
ignore `F401` error in all python files in `src/generated`.
Thus ale has to run flake8 from project root where .flake8 config
is placed otherwise glob won't match linted file.
@kalekseev
Copy link
Contributor Author

Hi @w0rp, I updated the pr with changes you requested.

and make sure we fall back on setting the CWD to the directory the file is in, in the Vader tests.

that implicitly tested by unrelated tests, because project root is default now and by default tests run on a file that doesn't have project mark in the parent dirs. If you prefer to have an explicit test for that case let me know.

@w0rp w0rp merged commit 31942c9 into dense-analysis:master Aug 31, 2020
@w0rp
Copy link
Member

w0rp commented Aug 31, 2020

Cheers! 🍻

@rafikdraoui
Copy link

Thanks! I've been using something similar to what's in this PR for a while now. I'm happy to be able to simplify my vimrc and use the default ale linter again!

The fact that per-file-ignores doesn't handle paths properly is a bug that is being tracked by flake8: https://gitlab.com/pycqa/flake8/-/issues/517.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants