[beta] How to publish SL4 with breaking changes (PythonLinter)

[braver]

cc @kaste, re #723

We're about to break compatibility with the python-based linters. I don't want to drag around legacy code and backwards compatibility in this repo if I can help it, so a decision needs to be made on how to go about this without simply breaking everything (which would be not good).

Two ideas I've come up with so far.

1. Publish SL4 not as a package itself but as a dependency

This is a pretty new possibility and it would be a good fit. SublimeLinter doesn't do anything by itself anyway.

Good:

• linters can be upgraded one by one
• we can publish it right now and it can be made to work side by side SL3 for linters that aren't upgraded yet

Bad:

• all linters will have to be upgraded, even though the breakage is only in a handful python linters
• not all linters are in the SL org, so some might never be upgraded at all
• if you use old linters you don't get the shiny new SL4
• or, perhaps worse, you get both SL3 and SL4 in a completely confusing mix

So, unless we're able to really push all 7 pages worth of linters forwards in a short period of time, or unless we build a time machine, this sounds really nice but impossible.

2. Version check in the linter

We could make the python-based linters forwards-compatible. We could edit their linter.py to check if they're running on SL3 and SL4 and have an implementation for both.

Good:

• we keep the backwards compatibility out of SL4 and force a move on the linter plugins
• most python-based linters are in the org so we can prepare them right now
• linters are tiny, having a little "if SL.version is 4" switch in there seems manageable to me

Bad:

• we may need to check on linters outside the org and preparing them might need coordinating
• we need to schedule a time after the SL4 release to clean up the SL3 code in multiple repo's

I think this might work because we can do all of it behind the scenes, while the SL4 release still forces the move forwards. We really need to drop the old behaviour so let's use that momentum.

---

More ideas?

[kaste]

The changes I did are meant to be forwards compatible. 😒 - 🐛 s

Following status:

• https://github.com/SublimeLinter/SublimeLinter-pylint works 👍

note: overrides internal build_args, should use cmd() instead

• https://github.com/SublimeLinter/SublimeLinter-pycodestyle works 👍

note: remove cruft after SL4
PR: SublimeLinter/SublimeLinter-pycodestyle#1

• https://github.com/SublimeLinter/SublimeLinter-pep8 works 👍

note: remove cruft after SL4
PR: SublimeLinter/SublimeLinter-pep8#20

• https://github.com/SublimeLinter/SublimeLinter-pydocstyle works 👍

note: remove cruft after SL4
PR: SublimeLinter/SublimeLinter-pydocstyle#18

• https://github.com/SublimeLinter/SublimeLinter-pyyaml 👎

note: ❓ uses yaml lib internally, and parses its tracebacks :WTF: don't know if yaml has a cli

• https://github.com/SublimeLinter/SublimeLinter-pyflakes works 👍

note: remove cruft after SL4
PR: SublimeLinter/SublimeLinter-pyflakes#8

[kaste]

https://github.com/SublimeLinter/SublimeLinter-json and https://github.com/SublimeLinter/SublimeLinter-annotations were broken, now fixed.

[braver]

Well that's even better then. Awesome. So, we have to go and find all python-based linters and prepare them, als you already did elsewhere? I hit a few that aren't in the org, so I want to set up some analytics on the linters that are out there so we can get a good look at what we're touching. May be useful for other refactors as well.

SublimeLinter-pyyaml

There are alternatives like SublimeLinter-contrib-yamllint, which sounds like a better and more useful approach to linting yaml. We could consider deprecating -pyyaml in favour of something else if it doesn't look upgradable.

[kaste]

@braver Can you write a script that checks out plugins and collects information about what methods they override. If possible also what API calls on self they make.

[braver]

Good idea, sounds like a fun project. I'll have to see what's within my abilities here though.

[kaste]

For JSON: using eslint or https://github.com/zaach/jsonlint (Atom uses this) is probably a better choice

For YAML: https://github.com/rasshofer/yaml-lint is probably the better choice. Or: https://github.com/adrienverge/yamllint

• https://github.com/SublimeLinter/SublimeLinter-rst 👎

Use https://github.com/twolfson/restructuredtext-lint instead

• https://github.com/fredcallaway/SublimeLinter-contrib-mypy 👍

note: Should not override build_cmd bc it is internal. Cannot benefit from the new features the new python linter provides (pipenv, executable) otherwise. [Basically their build_cmd is a no-op for SL4]

• https://github.com/SublimeLinter/SublimeLinter-bandit 👍

[kaste]

https://github.com/thomasmeeus/SublimeLinter-contrib-yamllint is python based and should work. 👍

npm's yaml-lint is probably better, but since there are already alternatives ➡️ we should deprecate our own yaml linter.

[braver]

we should deprecate our own yaml linter

Excellent, sounds like a plan.

[kaste]

FWIW @braver all linters can be found https://github.com/SublimeLinter/package_control_channel/blob/master/contrib.json and https://github.com/SublimeLinter/package_control_channel/blob/master/org.json

From there you get a git link, try to fetch the linter.py, try to regex [ ]{4}def (<name>)\(self. For python this might be enough.

[braver]

Thanks, I'll try something like that. Am already running into repos that don't exist anymore, so this is already useful :)

[FichteFoll]

Note that #653, which concerns all linters and not just Python, could potentially introduce even more breaking changes, depending on how it is tackled.

However, replacing the Python linter is the biggest and most significant change. Once inline loading is gone, we can look into streamlining the process for different virtualenv types and potentially adjusting the API for this. Maybe a task for SL5 though.

[FichteFoll]

Oh, I almost forgot: SublimeLinter is not a candidate for being deployed as a dependency because it rightfully provides user-facing commands and interfaces with ST (event listeners, commands, keybindings, etc).

Dependencies are only for Python stuff.

[braver]

Dependencies are only for Python stuff.

Oh, you're right. Good, that's off the table then.

#653 Runs through a lot of annoyances, not all of which related to the way linter executables are found, and several which are already resolved. E.g. we stop advocating the use of sublimelinterrc and inline settings at all and especially for linters that don't need them, and all python linters plugins now respect tox.ini and similar config files.

I like the rigidity of the proposal, but I'm not 100% sold on it. I think explicitly setting an executable should be what you use if the automagic doesn't work. I believe for the vast majority of users the current overall approach does actually work. So, I really feel like the work of @kaste is on the right track and don't necessarily see the need for similar big changes for non-python-based linters. The eslint linter is the only one I know that seems to have a bunch of weird setup issues, but the linter itself doesn't always work the way you expect and it attracts a lot of inexperienced web developers... eh, by which I mean, there are a lot of those (I used to be one), not necessarily that they ask dumb questions (there are no dumb questions etc) 😉

[kaste]

I don't understand #653, it is basically a rant bc flake8 didn't work for a week or two. SublimeLinter is already about finding executables. It uses package.json, pipenv, PATH

The riskiest things we can do or have to do, is decoupling the different systems. linter.py is too big. And the surface area for the plugin authors is too big. We have no unit tests, and a grown system for maybe 5 years, and none of the methods have a _ in front of them indicating there are private.

We can and should move lint_view out of Linter. Some other methods would follow lint_viewnaturally.

I'm also would like to see that the actual linter only calls the linter and parses its output. It shouldn't be concerned with drawing.

[kaste]

What's a good filename for all the static functions like lint_view?

[braver]

Luckily abuse of that surface area is pretty limited. It's not really possible to reduce it, linters can import anything they and we can't do much about it, but I'm not against improving the organisation of the codebase as long as we put in some effort to fix what we break. I'd like to see that as a separate phase after we merge #723 though, which I hope to be able to do some time next week.

[FichteFoll]

I think #653 ist just a bit too loaded and covers too many areas, some if which I agree with and some I don't necessarily. I guess I should sum this up with "we should try to find a linter executable in common virtual environment paths but allow an explicit override in case that fails or doesn't procude the desired result", which is what the new PythonLinter accomplishes, as far as I understand. The rest is details, that I'm not involved enough with to make a call, or similar.

This probably wouldn't break compatibility as much as the new PythonLinter though, so we can probably put that off until the next minor release or something.

(Oh, I think I also mostly wanted to refer to my comment there since this is pretty much the only thing I remember from that issue. 😅 )

[braver]

Linter plugins using PythonLinter (check means @kaste reports it working):

• bandit
• chktex
• contrib-bootlint
• contrib-cheetah-flake
• contrib-frosted
• contrib-mypy (should not override build_cmd)
• contrib-serplint
• contrib-sqflint
• contrib-yamllint
• flake8 (this one might need some coordination?)
• pep8 (possibly deprecate for pycodestyle)
• pydocstyle
• pyflakes
• pylint
• pyyaml (deprecating this one for contrib-yamllint)

[kaste]

• SublimeLinter/SublimeLinter-rst (Use twolfson/restructuredtext-lint instead?)
• chktex
• https://github.com/thomas-lebeau/SublimeLinter-contrib-bootlint
• https://github.com/jdb8/SublimeLinter-contrib-cheetah-flake
• https://github.com/Shura1oplot/SublimeLinter-contrib-frosted
• https://github.com/beaugunderson/SublimeLinter-contrib-serplint
• https://github.com/LordGolias/SublimeLinter-contrib-sqflint

flake8 should just work bc it has its own implementation. I have a clean version on my computer running. Didn't I PR already?

[braver]

@kaste the result of running grep ' def .*self' over all linter.py files of all linter plugins:

list.txt

[kaste]

Thanks for the list.

• https://github.com/jfcherng/SublimeLinter-contrib-gcc/blob/master/linter.py will break.

They try to fix SL3 behavior:

    # We would like to bind "executable" later in cmd(self), but
    # if "executable" is not found here, this linter won't be activated.
    # The following if-branch just makes sure an "executable" could be found.
    if sublime.platform() == 'windows':
        # Windows OS would have "cmd" (or "explorer") binary in its PATH
        executable = 'cmd'
    else:
        # A non-Windows OS would have "cat" binary in its PATH?
        executable = 'cat'

I fixed this too, so the correct API now is to not set executable at all or set executable = None.

The user story: A user wants to lazy evaluate and find/locate which executable to run. The 'correct' way for SL3 was, to override the crazy can_lint static method. Note that all our base_linter do this already except the ruby_linter. (The ruby_linter evaluates the ruby environment on startup, and is thus more likely to be a victim of #670.) But this override was non-trivial. E.g. for node_linter

https://github.com/SublimeLinter/SublimeLinter3/blob/aa820c994a9d2a0fe3665aa129d8f06165f64042/lint/node_linter.py#L249-L270

Honestly, you'd had to copy-paste this to get it right. Our base linters all copy pasted here.

For SL4, I wanted a simpler solution. If you want a dynamic executable, you don't set executable and define cmd as a method/callable.

[braver]

Is there a change we can propose in contrib-gcc to prepare it?

[kaste]

An ugly one. You basically would use what the original SublimeLinter author wanted you to use. You implement can_lint as above via copy paste. And then you implement context_sensitive_executable_path

def context_sensitive_executable_path(self, cmd):
	return True, cmd[0]

I think that's the intended API usage. Originally SublimeLinter was designed to have static executables. They were resolved in can_lint once (for efficiency). can_lint set executable_path. This didn't work out
so they introduced context_sensitive_executable_path which is a dynamic or lazy executable_path as the name suggests.

[braver]

Ok, we can at least open an issue there so they can anticipate the upcoming changes. Edit: posted SublimeLinter/SublimeLinter-gcc#5

Too bad there isn't a SL-based alternative for rst, but it's also unmaintained since 2014. I'll put something in the readme (edit: Done), perhaps someone turns up who wants to go out and fix things.

[braver]

Flake8 at master does result in breakage on my machine. I'll just publish a prerelease there too.

[braver]

Ok, I think we're good here. Thanks everyone!

[zspitzer]

SublimeLinter-contrib-CFLint is broken
ckaznocha/SublimeLinter-contrib-CFLint#23