-
Notifications
You must be signed in to change notification settings - Fork 98
Add swiftly to walkthrough #2078
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: main
Are you sure you want to change the base?
Changes from 10 commits
b423536
e8352a7
4e89817
a6701d1
41bbaa7
7374c67
93f3706
3891b41
88ccb83
7fdd3b6
09c7e57
fe340b1
96da02a
fcc4759
27440b0
8923fc4
c011d4f
bdb68d3
531c30c
27973c8
a65622c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import * as child_process from "child_process"; | ||
| import { promisify } from "util"; | ||
|
|
||
| const exec = promisify(child_process.exec); | ||
|
|
||
| export async function swiftInstalled(): Promise<boolean> { | ||
| try { | ||
| await exec("swift --version"); | ||
| return true; | ||
| } catch (error) { | ||
thePianoKid marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ import * as commands from "./commands"; | |
| import { resolveFolderDependencies } from "./commands/dependencies/resolve"; | ||
| import { registerSourceKitSchemaWatcher } from "./commands/generateSourcekitConfiguration"; | ||
| import { handleMissingSwiftly } from "./commands/installSwiftly"; | ||
| import { swiftInstalled } from "./commands/swiftInstalled"; | ||
| import configuration, { | ||
| ConfigurationValidationError, | ||
| handleConfigurationChangeEvent, | ||
|
|
@@ -80,6 +81,10 @@ export async function activate( | |
| checkForSwiftlyInstallation(context, contextKeys, logger); | ||
| const swiftlyCheckElapsed = Date.now() - swiftlyCheckStartTime; | ||
|
|
||
| const swiftLangCheckStartTime = Date.now(); | ||
| await checkForSwiftLangInstallation(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that we can render a checkmark beside the first step of the walkthrough when |
||
| const swiftLangCheckElapsed = Date.now() - swiftLangCheckStartTime; | ||
|
|
||
| const toolchainStartTime = Date.now(); | ||
| const toolchain = await createActiveToolchain(context, contextKeys, logger); | ||
| const toolchainElapsed = Date.now() - toolchainStartTime; | ||
|
|
@@ -89,23 +94,28 @@ export async function activate( | |
| // properly configured. | ||
| if (!toolchain) { | ||
| // In order to select a toolchain we need to register the command first. | ||
| const subscriptions = commands.registerToolchainCommands(undefined, logger); | ||
| const chosenRemediation = await showToolchainError(); | ||
| subscriptions.forEach(sub => sub.dispose()); | ||
| context.subscriptions.push( | ||
| ...commands.registerToolchainCommands(undefined, logger, context.extensionPath) | ||
| ); | ||
|
|
||
| // If they tried to fix the improperly configured toolchain, re-initialize the extension. | ||
| if (chosenRemediation) { | ||
| return activate(context); | ||
| } else { | ||
| return { | ||
| workspaceContext: undefined, | ||
| logger, | ||
| activate: () => activate(context), | ||
| deactivate: async () => { | ||
| await deactivate(context); | ||
| }, | ||
| }; | ||
| } | ||
| // Show the error dialog asynchronously without blocking activation. | ||
| // This prevents a UI deadlock where the walkthrough buttons can't work | ||
| // because activation is blocked waiting for the user to dismiss the dialog. | ||
| void showToolchainError().then(chosenRemediation => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found this was the only way to get the "Install Swift" button to work while there was no Swift toolchain installed (which is the most important time for that button to work!)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Update] @matthewbastien has a PR up that will reduce extension activation time so that it's not blocking the toolchain popup. Once that goes in, I'll rebase and refactor so that the toolchain installation doesn't block the extension activation. |
||
| // If they tried to fix the improperly configured toolchain, re-initialize the extension. | ||
| if (chosenRemediation) { | ||
| void activate(context); | ||
| } | ||
| }); | ||
|
|
||
| return { | ||
| workspaceContext: undefined, | ||
| logger, | ||
| activate: () => activate(context), | ||
| deactivate: async () => { | ||
| await deactivate(context); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| const workspaceContextStartTime = Date.now(); | ||
|
|
@@ -117,7 +127,11 @@ export async function activate( | |
| context.subscriptions.push(new SwiftEnvironmentVariablesManager(context)); | ||
| context.subscriptions.push(SwiftTerminalProfileProvider.register()); | ||
| context.subscriptions.push( | ||
| ...commands.registerToolchainCommands(workspaceContext, workspaceContext.logger) | ||
| ...commands.registerToolchainCommands( | ||
| workspaceContext, | ||
| workspaceContext.logger, | ||
| context.extensionPath | ||
| ) | ||
| ); | ||
|
|
||
| // Watch for configuration changes the trigger a reload of the extension if necessary. | ||
|
|
@@ -184,7 +198,7 @@ export async function activate( | |
|
|
||
| const totalActivationTime = Date.now() - activationStartTime; | ||
| logger.info( | ||
| `Extension activation completed in ${totalActivationTime}ms (log-setup: ${logSetupElapsed}ms, pre-toolchain: ${preToolchainElapsed}ms, toolchain: ${toolchainElapsed}ms, swiftly-check: ${swiftlyCheckElapsed}ms, workspace-context: ${workspaceContextElapsed}ms, subscriptions: ${subscriptionsElapsed}ms, workspace-folders: ${workspaceFoldersElapsed}ms, final-steps: ${finalStepsElapsed}ms)` | ||
| `Extension activation completed in ${totalActivationTime}ms (log-setup: ${logSetupElapsed}ms, pre-toolchain: ${preToolchainElapsed}ms, toolchain: ${toolchainElapsed}ms, swiftly-check: ${swiftlyCheckElapsed}ms, swift-lang-check: ${swiftLangCheckElapsed}, workspace-context: ${workspaceContextElapsed}ms, subscriptions: ${subscriptionsElapsed}ms, workspace-folders: ${workspaceFoldersElapsed}ms, final-steps: ${finalStepsElapsed}ms)` | ||
thePianoKid marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ); | ||
|
|
||
| return { | ||
|
|
@@ -406,6 +420,15 @@ function findSwiftVersionFilesInWorkspace(): Promise<string[]> { | |
| ).then(results => results.reduceRight((prev, curr) => prev.concat(curr), [])); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if swift --version works, and updates the "swiftInstalled" context variable | ||
| */ | ||
| async function checkForSwiftLangInstallation() { | ||
| const isInstalled = await swiftInstalled(); | ||
| // The welcome page checks the swiftInstalled context variable and renders a check mark if true | ||
| await vscode.commands.executeCommand("setContext", "swiftInstalled", isInstalled); | ||
| } | ||
|
|
||
| async function createActiveToolchain( | ||
| extension: vscode.ExtensionContext, | ||
| contextKeys: ContextKeys, | ||
|
|
||
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 this, I was thinking the first "Install Swift" step could also have an Install Swiftly and Swift Toolchain button instead of being told to manually install from swift.org. Possibly can detect whether the user has an existing toolchain (or Xcode installed) and if they don't they can click a button and have the extension set everything up?