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

exclude setting doesn't work when passing full path #72

Closed
asottile opened this issue Apr 3, 2021 · 6 comments
Closed

exclude setting doesn't work when passing full path #72

asottile opened this issue Apr 3, 2021 · 6 comments

Comments

@asottile
Copy link
Member

asottile commented Apr 3, 2021

In GitLab by @arufuredosan on Jan 23, 2015, 13:10

On a setup.cfg file that has:

[flake8]
exclude = tests

And a project that has the following layout:

project/
    tests/
        test_foo.py

If on the project directory and I run:

flake8 .

Then the configuration will correctly kick in and tests will not be picked up.

However, if we run:

flake8 tests/test_foo.py

Then that file gets linted.

Although it is non-obvious why someone ignoring tests would pass the file that is actually in the ignored
directory, it is how the khuno.vim plugin works. When on a given file (like test_foo.py) the plugin
will call flake8 with the full path to report back the linting to the editor.

Reference issue on khuno tracker: alfredodeza/khuno.vim#20

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @sigmavirus24 on Jan 23, 2015, 19:38

This expected and encouraged behaviour. The use case where someone explicitly does flake8 tests/test_foo.py is where a set of files is known to not yet be passing flake8 so by default the project excludes those files. If a user, however, wants to begin to make the file pass flake8, they would have to remove the line excluding those files.

In the case of an entire directory, this is impractical and wrong. As a compromise passing the file will ignore the exclude directive assuming the user actually wants to lint that file.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @arufuredosan on Jan 24, 2015, 06:10

The "impractical and wrong" case is not documented as such and the behavior is now relied upon for other tools that consume flake8.

There is also nothing in the documentation that clearly states that this behavior should be expected (e.g. "exclude rules will be bypassed if a path that matches the rule is passed in explicitly in the command line").

Even if there isn't motivation to change current behavior, I would urge you to reconsider and re-open this ticket to at least address the documentation part, so I can point users to why the linter is not working as they expect (as you can see in the Github link in the subject).

In an ideal scenario, some other flag that would allow strict exclude rules (that would be obeyed even if a path is passed in) would be implemented so that editor tools like khuno can take advantage of it.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @sigmavirus24 on Jan 24, 2015, 11:15

The "impractical and wrong" case is not documented as such and the behavior is now relied upon for other tools that consume flake8.

As far as I know khuno is the only one relying on this. I use a lot of tools that use flake8 and haven't seen anyone else expecting this behaviour.

I would urge you to reconsider and re-open this ticket to at least address the documentation part

I'm always more than happy to accept updates to the documentation. This issue isn't about documentation though so it doesn't need to be reopened to address that.

so I can point users to why the linter is not working as they expect (as you can see in the Github link in the subject).

You can point users to a closed issue just as easily as an open one. They don't disappear because they're closed.

In an ideal scenario, some other flag that would allow strict exclude rules

The exclude rules are strict. The feature you want is exclude rules that take precedence over explicit user input. That's a counter-intuitive user experience in my opinion.

Looking at Khuno's README (there doesn't seem to be any other documentation), you already provide a way to configure (via khuno) error codes to ignore and maximum line-length. If you were solely relying on Flake8 for that functionality (by forcing the user to place all of this in a config file) then I'd understand your concern, but it would seem you could just as easily have the user specify files to exclude and apply it yourself.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @sigmavirus24 on Jan 24, 2015, 11:24

Also, for what it's worth, this functionality is provided by pep8. There's a (somewhat) related issue open there currently that may also be of interest to you.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @arufuredosan on Jan 24, 2015, 17:20

By 'pointing users to why the linter is not working' I meant at least a portion in the documentation where it states that this is expected behavior, not a closed/open issue.

You can't say something is strict (like the exclude rule) and then have an override. If it is strict, then it should be strict, otherwise it isn't. By definition, strict means obeying a rule exactly as it is. This is not doing that, therefore it isn't strict.

The only way khuno provides overrides is by allowing a user to define options that are passed into flake8 in the command line. This is not equal to the issue reported as it relies on machinery within flake8 because it is a per project setting which khuno doesn't have.

Any changes in setup.cfg or similar cannot be handled by the VimL plugin.

The only workaround for khuno I can think of would be to allow to specify global overrides and apply those, but that would be annoying if someone relies for per-project settings (which is the case here).

If you believe some feature of flake8 that is used in a different way than expected, in a "counter-intuitive" way then mention it, make a note, a warning, anything that anyone can then refer to so that they can have a better use of the tool.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @sigmavirus24 on Jan 24, 2015, 18:57

By 'pointing users to why the linter is not working' I meant at least a portion in the documentation where it states that this is expected behavior, not a closed/open issue.

Right. I already said the project would accept a contribution that fixed its documentation.

You can't say something is strict (like the exclude rule) and then have an override. If it is strict, then it should be strict, otherwise it isn't. By definition, strict means obeying a rule exactly as it is. This is not doing that, therefore it isn't strict.

It's as strict as every other linter. pep8 has the same behaviour (but as I've already mentioned, that's because pep8's logic powers flake8). pylint behaves the same way. If you create a .pylintrc with ignore=somefile.py and then you do pylint path/to/somefile.py it will evaluate it regardless.

Any changes in setup.cfg or similar cannot be handled by the VimL plugin.

I never told you to update the setup.cfg. If users want to ignore tests/ however, you can check the path name and not run flake8 on it. That's certainly doable by a VimL plugin.

If you believe some feature of flake8 that is used in a different way than expected, in a "counter-intuitive" way then mention it, make a note, a warning, anything that anyone can then refer to so that they can have a better use of the tool.

It's counter-intuitive because no other tool provides such an option. In fact, I'll go so far as to say it's an undesirable feature for everyone except khuno. What user will realistically have a config file excluding a directory/group of files and then want to do flake8 --ignore-what-I-say-and-ignore=tests/test_something.py tests/test_something.py. There's no use-case for that. Even so, it wouldn't help khuno unless it resolves to parse the potential config for exclude paths.

This discussion is becoming cyclic so if you want to open a separate issue to document the behaviour, or even send the patch yourself, that's great, but I won't be responding.

@asottile asottile closed this as completed Apr 3, 2021
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

No branches or pull requests

1 participant