SublimeLinter / SublimeLinter-flake8

SublimeLinter plugin for python, using flake8.
MIT License
184 stars 28 forks source link

Improve compatibility with multiple flake8 plugins #109

Closed ROpdebee closed 5 years ago

ROpdebee commented 5 years ago

This PR adds compatibility with multiple flake8 plugins, including flake8-aaa, flake8-pie and possibly others (yet to discover).

Concrete changes proposed:

Please Note: Your entry point does not need to be exactly 4 characters as of Flake8 3.0. Consider using an entry point with 3 letters followed by 3 numbers (i.e. ABC123 ).

Meaning that as of flake8 3, it's possible to have error codes with more than one letter. There's a couple plugins that make use of this feature, e.g. flake8-aaa which uses AAA01 - AAA99, and flake8-pie, which uses PIE781, PIE782.

kaste commented 5 years ago

Hi, that's cool! The ':' from PIE is .. well .. a bug. We should probably tell them?

Otherwise the regex stuff is 👍 . The --std-display-name is probably totally breaking 😢 bc flake8 v2 will just complain: unknown arg.

Not completely sure what happens when you try this with unnamed/unsaved ad-hoc buffers, does flake complain bc it expects a filename?

ROpdebee commented 5 years ago

I've tested the --std-display-name on unsaved files and it appears to be working fine, I'm guessing passing ${file} tells SublimeLinter to substitute the original filename on the unsaved file? For new files that haven't been saved to any file yet (the "untitled" variant), it just substitutes with nothing:

cat <buffer 1270> | /path/to/flake8 --format default --stdin-display-name --doctests -

Seems like flake8 doesn't complain about having an empty value but I guess there's better ways to handle this. Although that wouldn't allow flake8-aaa and similar plugins to recognise the files, there really is no way that they would be able to -- there is no filename.

As for flake8 v2, I guess the flag should just not be included in that case. Maybe a version check would be the best option in this case? Although I'm not entirely sure what would be the best way to approach this.

kaste commented 5 years ago

As there is currently no way to do the version check, I would say we just pull the regex changes. Users can still set "args": "--stdin-display-name ${file}" manually.

ROpdebee commented 5 years ago

The guys over at https://github.com/fredcallaway/SublimeLinter-contrib-mypy used to implement a version check but removed it in a recent commit: https://github.com/fredcallaway/SublimeLinter-contrib-mypy/commit/aef0653559d68edb028cdc87b13ab806e18ec968 They used to check the version whenever the linter ran, which is something we do not want, and most likely wouldn't work anyway. But perhaps it would be possible to check the version during plugin init, set an attribute, and turn cmd into a property that includes that extra arg if the version is >= 3.0.0 (or whatever the version in which the arg was introduced)? The main issue I can see happening here is if the version changes after the plugin is initialized, because of an update, venv switch, project switch or changing the settings.

Otherwise I'll revert the arg commit, rebase, and add a note on compatibility with some plugins in the README, if that's okay with you.

kaste commented 5 years ago

Ah, forget about that, this is my own commit. Difficult to cache this if the executable is pipenv run mypy or similarly py.exe -2 -m mypy. Just on init is not good enough, we at least must check per project/folder. What would be a good cache tuple (working_dir, cmd-str). If a simple executable is given probably (executable-modifiedtime, executable-path).

I actually pulled and pushed the 'regex' commits already. A note for the README would be good, though.

ROpdebee commented 5 years ago

Seems like an awful lot of work when setting args manually works just fine, and can be overridden on a per-project basis.

I've committed a change to the README to add the compatibility note.

kaste commented 5 years ago

I also pulled the README change. I'm closing this one since the version thing would take more commits anyway. We should at least wait until SublimeLinter provides a get_executable() method bc the mypy way to get the executable was basically a happy path approach. (I generally don't like caching if we don't have proper and sound invalidation rules.)