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

The default swiftlint.path value is invalid on Windows #67

Closed
tristanlabelle opened this issue Aug 11, 2023 · 14 comments
Closed

The default swiftlint.path value is invalid on Windows #67

tristanlabelle opened this issue Aug 11, 2023 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@tristanlabelle
Copy link
Collaborator

At The Browser Company, we're fixing SwiftLint to run on Windows and have produced builds of it. This allows us to use the vscode-swiftlint extension almost out-of-the-box, except that the default configuration fails to launch SwiftLint because the executable is launched via /usr/bin/env, which does not exist on Windows.

"default": [
"/usr/bin/env",
"swiftlint"
],

              "default": [
                "/usr/bin/env",
                "swiftlint"
              ],

Could we change the default to be only "swiftlint" to make it valid on all platforms? As needed, the /usr/bin/env could be prepended programatically on unix platforms in the extension code.

@vknabel vknabel added the bug Something isn't working label Aug 11, 2023
@vknabel vknabel self-assigned this Aug 11, 2023
@vknabel
Copy link
Owner

vknabel commented Aug 11, 2023

Hi @tristanlabelle, thanks for your suggestion! As I don't have access to a windows machine, I am not able to test the current build, but I created a beta build for this extension including your fix for windows. Could you give it a try?
1.8.4-beta.0

@tristanlabelle
Copy link
Collaborator Author

tristanlabelle commented Aug 14, 2023

@vknabel I'm having issues with the vsix... I wonder if my VS Code is in a bad state 🤔

image

image

I installed it from the "Install from VSIX" command here:
image

@vknabel
Copy link
Owner

vknabel commented Aug 16, 2023

That's weird. You did nothing wrong. Not sure why it isn't working though.
I created a new build with bumped dependencies. Eventually this solves your issue.
1.8.4-beta.1

@tristanlabelle
Copy link
Collaborator Author

My bad, I had swiftlint.enable = false in my user settings. The fix does not work, however. It's still trying to use /usr/bin/env.

image

@vknabel
Copy link
Owner

vknabel commented Sep 16, 2023

Sorry for the delay. I've taken some time off.

It seems like shell commands need to be executed differently on windows. Does this release solves your issue?
https://github.com/vknabel/vscode-swiftlint/releases/tag/1.8.4-beta.3

@tristanlabelle
Copy link
Collaborator Author

@vknabel I will try it out during the week (no computer access right now), but I was just thinking that a fix to this problem would be to implicitly prepend /usr/bin/env in the code if running on unix systems and the swiftlint.path contains no slashes (ie it is the unqualified name of an executable). Windows' CreateProcess function will look in the %Path% by default so that will work there too.

It make a bit more sense too because swiftlint.path is currently more of a command line prefix than a path.

@vknabel
Copy link
Owner

vknabel commented Sep 16, 2023

Back in time I did use a different function in the viscose extension which (in theory) would prefix everything with /usr/bin/env. But in some cases it did not work properly and it did not respect the local PATH variables of the user unless started from the shell itself.

On windows I now use the old way with a thin layer of compatibility. I hope this helps.

But I agree swiftlint.path is actually a prefix (an could in theory be set to swift run swiftlint etc).

@tristanlabelle
Copy link
Collaborator Author

I implemented this fix and it works on my machine: #73 . I also want to propose #72

@vknabel
Copy link
Owner

vknabel commented Oct 1, 2023

Thanks a lot for your work and enthusiasm! Sadly I am currently unable to provide the deserved time and energy to this project and the other two extensions (vscode-swiftformat and vscode-apple-swift-format).

I'd be happy if you are willing to support me and in the long term eventually take them over.

@vknabel
Copy link
Owner

vknabel commented Oct 1, 2023

Fixes have been released

@vknabel vknabel closed this as completed Oct 1, 2023
@tristanlabelle
Copy link
Collaborator Author

I'd be happy if you are willing to support me and in the long term eventually take them over.

Supporting this extension is actually interesting to The Browser Company, where I work. I'll be in touch when I have more info!

@oskargargas
Copy link

At The Browser Company, we're fixing SwiftLint to run on Windows and have produced builds of it. This allows us to use the vscode-swiftlint extension almost out-of-the-box (...)

@tristanlabelle you mentioned that you have produced Windows builds of SwiftLint. Are they available somewhere? I've tried @compnerd Windows fork but to no success (build fails because of SourceKitten)

@compnerd
Copy link

compnerd commented Dec 27, 2023

@oskargargas realm/SwiftLint#5030 is the patch set for the Windows support for SwiftLint, and jpsim/SourceKitten#769 is the patch set for SourceKitten. There is a prebuilt binary at https://github.com/thebrowsercompany/swift-build/releases/tag/SwiftLint-DEVELOPMENT-SNAPSHOT-2023-08-12-a which is the most recent one, but if there is a need, we should be able to trigger a newer build as well.

I'd love help to get the changes for SourceKitten through :)

@oskargargas
Copy link

oskargargas commented Dec 28, 2023

Thanks @compnerd for the link. Works like a charm. (+/- CPU usage, but it's probably expected on apparently 10 year old CPU...)

I'd love help to get the changes for SourceKitten through :)

I'll take a look if only time allows. Have a few more days off but also some things planned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants