SublimeLinter / SublimeLinter-shellcheck

This linter plugin for SublimeLinter provides an interface to shellcheck.
MIT License
210 stars 20 forks source link

Add Windows support using WSL #9

Closed spikespaz closed 6 years ago

spikespaz commented 6 years ago

Update code to use basic Linux Subsystem Update README.md with small guide to utilize WSL

kaste commented 6 years ago

Very interesting. Does that mean 'wsl' spawns a subshell? Is there an ENV set on Windows that indicates walk support?

kaste commented 6 years ago

Autocorrect: walk -> wsl

spikespaz commented 6 years ago

WSL must simply be enabled, I guess depending on what you are trying to do with the information, checking if where wsl returns a valid path would be sufficient for determining support. See the link to Microsoft's guide that I added in the readme.

This is also confirmed to be working for me.

kaste commented 6 years ago

The guide did not mention wsl, so I had to ask.

spikespaz commented 6 years ago

Oh. Well, I don't remember how I knew about wsl, I just sort of saw this on package control and thought "oh hey this is possible on windows" and implemented it in 2 minutes.

spikespaz commented 6 years ago

@kaste

I had an idea, I could close this PR and add a little code to make sure that WSL is enabled and installed, then submit a new one. Or add to this. Should I do that, or is it fine the way it is assuming the user read the instructions?

Also, if I add the check, how should I tell the user that they don't have WSL enabled?

kaste commented 6 years ago

Now, I would be very interested in a general way. (Actually to put it into SublimeLinter core, eventually, maybe.)

You don't have to close this one. This one is #9, so we're surely not getting confused here. 😄

You could also outline the idea first - really depends on how much code the real solution would be. (Also for SL4, you would inform the user just using normal python logging. Either a warning or as an error.)

spikespaz commented 6 years ago

Alright that should be the last thing, I added an OSError when wsl is not found. That either means that something (OS, ST, or Python) is 32-bit or that the user hasn't enabled the subsystem.

kaste commented 6 years ago

Good point with the simple which call. wsl is installed in the system folders anyway. So they can't be no PATH issues. 👍

spikespaz commented 6 years ago

@kaste Is that good? Or do you want to explicitly use shutil.which() as well?

Also, why not use a deep import for functions? I have never heard of this convention, and no linter has ever complained.

kaste commented 6 years ago

who not use a deep import for functions

If you don't import deep, the python import mechanics are a bit different and it's easier to avoid cyclic imports. But honestly, I only mentioned that bc you forgot to call platform(), so I had to ask for an edit anyway.

kaste commented 6 years ago

🎉 🕺