-
Notifications
You must be signed in to change notification settings - Fork 67
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
feat: secondary quick pick for selecting Swift Version with runSwiftScript
command
#1476
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for the contribution! I think this is looking good so far. The only change I'd like to see, and @adam-fowler mentioned it recently as well in the original issue, is the addition of a setting that would allow users to bypass the quickPick dialog and always use the same language mode.
The setting should be defined in the package.json of the extension, and exposed through configuration.ts, probably at the workspace folder level so it can be defined per package as well as globally
Hi @plemarquand , thanks for the feedback! I have included some changes to enable setting something like |
package.json
Outdated
@@ -324,6 +324,15 @@ | |||
}, | |||
"markdownDescription": "Additional arguments to pass to `swift build` and `swift test`. Keys and values should be provided as individual entries in the list. If you have created a copy of the build task in `tasks.json` then these build arguments will not be propagated to that task." | |||
}, | |||
"swift.defaultSwiftVersion": { |
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.
This setting should be an enum
type, with the options "6", "5", and "Ask Every Run". enumDescriptions
should be provided for each option as well. I think the default value for the setting should be 6
.
You can see an example of an enum
style setting here: https://github.com/swiftlang/vscode-swift/blob/main/package.json#L557. This way the user can't pick an unsupported value, they'll get a warning in the JSON editor and they'll get a nice picker UI in the settings UI page.
Also I think the name defaultSwiftVersion
is too generic and may be confusing. This setting only applies when running Swift scripts but could be interpreted as some kind of global. I'd rename it to scriptSwiftLanguageVersion
.
src/configuration.ts
Outdated
@@ -328,6 +328,9 @@ const configuration = { | |||
.get<string[]>("buildArguments", []) | |||
.map(substituteVariablesInString); | |||
}, | |||
get defaultSwiftVersion(): number | undefined { |
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 type of this setting would also have to change to string
since the valid options are "6"
, "5"
and `"Ask Every Run"
Hi @plemarquand thanks for the review! I have made corresponding changes, let me know if you need anything else! |
@louisunlimited thanks for taking a look, but I don't see any new commits. Did the changes get pushed up? |
@plemarquand Oh my bad, should be up now! |
This PR tries to implement a secondary quick pick for selecting desired Swift Version for
Run Swift Script
command, and also enabling global settings for it in vscode workspace.Swift: Run Swift Script
always runs in Swift 5 language mode #1427