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.

Sign up for GitHub

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

Set swift.swiftSDK for target platforms #1390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Set swift.swiftSDK for target platforms #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] {
}
}),
// Note: This is only available on macOS (gated in `package.json`) because its the only OS that has the iOS SDK available.
vscode.commands.registerCommand("swift.switchPlatform", () => switchPlatform()),
vscode.commands.registerCommand("swift.switchPlatform", () => switchPlatform(ctx)),
vscode.commands.registerCommand(Commands.RESET_PACKAGE, () => resetPackage(ctx)),
vscode.commands.registerCommand("swift.runScript", () => runSwiftScript(ctx)),
vscode.commands.registerCommand("swift.openPackage", () => {
Expand Down
53 changes: 35 additions & 18 deletions src/commands/switchPlatform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@
//===----------------------------------------------------------------------===//

import * as vscode from "vscode";
import { DarwinCompatibleTarget, SwiftToolchain } from "../toolchain/toolchain";
import {
DarwinCompatibleTarget,
SwiftToolchain,
getDarwinTargetTriple,
} from "../toolchain/toolchain";
import configuration from "../configuration";
import { Version } from "../utilities/version";
import { WorkspaceContext } from "../WorkspaceContext";

/**
* Switches the target SDK to the platform selected in a QuickPick UI.
* Switches the appropriate SDK setting to the platform selected in a QuickPick UI.
*/
export async function switchPlatform() {
export async function switchPlatform(ctx: WorkspaceContext) {
const picked = await vscode.window.showQuickPick(
[
{ value: undefined, label: "macOS" },
Expand All @@ -29,28 +35,39 @@ export async function switchPlatform() {
{ value: DarwinCompatibleTarget.visionOS, label: "visionOS" },
],
{
placeHolder: "Select a new target",
placeHolder: "Select a new target platform",
}
);
if (picked) {
if (ctx.toolchain.swiftVersion.isLessThan(new Version(6, 1, 0))) {
vscode.window.showWarningMessage(
"Code editing support for non-macOS platforms is only available starting Swift 6.1"
);
}
// show a status item as getSDKForTarget can sometimes take a long while to xcrun for the SDK
const statusItemText = `Setting target platform to ${picked.label}`;
ctx.statusItem.start(statusItemText);
try {
const sdkForTarget = picked.value
? await SwiftToolchain.getSDKForTarget(picked.value)
: "";
if (sdkForTarget !== undefined) {
if (sdkForTarget !== "") {
configuration.sdk = sdkForTarget;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should keep configuration.sdk for toolchains less than 6.1 or if we should just avoid setting it entirely since it doesn't provide any code editing support from SourceKit-LSP. Also it seems like we'll need either configuration.sdk or configuration.swiftSDK set, but not both together.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should keep configuration.sdk for toolchains less than 6.1 or if we should just avoid setting it entirely since it doesn't provide any code editing support from SourceKit-LSP.

I'm leaning towards just hiding this command altogether on unsupported toolchain and OS combinations instead of showing a warning notification. The setting description in the package.json should probably have a warning about this as well.

Also it seems like we'll need either configuration.sdk or configuration.swiftSDK set, but not both together.

I'm not entirely sure of the difference between the two, but I'm guessing configuration.sdk is for older Swift versions that don't support the --swift-sdk option? In which case we could even change the behaviour of this command depending on the Swift version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, yes good points, seems like it only makes sense to really show the option for supported combinations (currently only Darwin-based platforms). Potentially in the future it would make sense to have a list of common target-triples for other platforms as well to enable convenient switching.

It doesn't look like setting configuration.sdk makes much difference for any code editing/SourceKit support functionality. Perhaps it did something for much older toolchains, but it doesn't seem to do much in terms of code editing support for 5.10 or 6 at least. Probably better to avoid setting it to something automatically to prevent confusion.

vscode.window.showWarningMessage(
`Selecting the ${picked.label} SDK will provide code editing support, but compiling with this SDK will have undefined results.`
);
} else {
configuration.sdk = undefined;
}
if (picked.value) {
// verify that the SDK for the platform actually exists
await SwiftToolchain.getSDKForTarget(picked.value);
}
const swiftSDKTriple = picked.value ? getDarwinTargetTriple(picked.value) : "";
if (swiftSDKTriple !== "") {
// set a swiftSDK for non-macOS Darwin platforms so that SourceKit-LSP can provide syntax highlighting
configuration.swiftSDK = swiftSDKTriple;
vscode.window.showWarningMessage(
`Selecting the ${picked.label} target platform will provide code editing support, but compiling with a ${picked.label} SDK will have undefined results.`
);
} else {
vscode.window.showErrorMessage("Unable to obtain requested SDK path");
// set swiftSDK to undefined for macOS and other platforms
configuration.swiftSDK = undefined;
}
} catch {
vscode.window.showErrorMessage("Unable to obtain requested SDK path");
vscode.window.showErrorMessage(
`Unable set the Swift SDK setting to ${picked.label}, verify that the SDK exists`
);
}
ctx.statusItem.end(statusItemText);
}
}
6 changes: 1 addition & 5 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<Api> {
event.affectsConfiguration("swift.SDK") ||
event.affectsConfiguration("swift.swiftSDK")
) {
// FIXME: There is a bug stopping us from restarting SourceKit-LSP directly.
// As long as it's fixed we won't need to reload on newer versions.
showReloadExtensionNotification(
"Changing the Swift SDK path requires the project be reloaded."
);
vscode.commands.executeCommand("swift.restartLSPServer");
}
})
);
Expand Down