-
Notifications
You must be signed in to change notification settings - Fork 82
Move 5.1 build system to a variant #207
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
base: master
Are you sure you want to change the base?
Conversation
So Windows can run either one Fixes SublimeText#204
| "windows": { | ||
| "cmd": ["powershell.exe", "-noprofile", "-ExecutionPolicy", "Bypass", "-file", "$file"], | ||
| } | ||
| "variants": [ | ||
| { | ||
| "name": "Windows PowerShell (<=5.1)", | ||
| "windows": { | ||
| "cmd": ["powershell.exe", "-noprofile", "-ExecutionPolicy", "Bypass", "-file", "$file"], | ||
| }, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"variants" doesn't need to be wrapped into "windows"
| "windows": { | |
| "cmd": ["powershell.exe", "-noprofile", "-ExecutionPolicy", "Bypass", "-file", "$file"], | |
| } | |
| "variants": [ | |
| { | |
| "name": "Windows PowerShell (<=5.1)", | |
| "windows": { | |
| "cmd": ["powershell.exe", "-noprofile", "-ExecutionPolicy", "Bypass", "-file", "$file"], | |
| }, | |
| }, | |
| ], | |
| }, | |
| "variants": [ | |
| { | |
| "name": "Windows PowerShell (<=5.1)", | |
| "windows": { | |
| "cmd": ["powershell.exe", "-noprofile", "-ExecutionPolicy", "Bypass", "-file", "$file"], | |
| } | |
| } | |
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way your version works, there's a "Windows PowerShell" build available here on Linux (but shouldn't be). I'll push a different variation and see what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then probably vice versa. What triggered me was duplicate "windows" entry, which I think may be redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll try it that way. What was wrong with the current state? Is the main pwsh one missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"variants" must be a top-level key. Hence your current revision, despite duplicating "windows" key, does not add the desired entry on Windows platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it seems, platform-sepcificly visible variants are not supported.
So Windows can run either one
Fixes #204