Skip to content

Improve Swiftly installation progress notification #1901

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

Merged
merged 1 commit into from
Oct 23, 2025
Merged

Improve Swiftly installation progress notification #1901

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
37 changes: 21 additions & 16 deletions src/commands/installSwiftlyToolchain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,36 @@ export async function installSwiftlyToolchainWithProgress(
await Swiftly.installToolchain(
version,
(progressData: SwiftlyProgressData) => {
if (
progressData.step?.percent !== undefined &&
progressData.step.percent > lastProgress
) {
const increment = progressData.step.percent - lastProgress;
if (progressData.complete) {
// Swiftly will also verify the signature and extract the toolchain after the
Copy link
Contributor

Choose a reason for hiding this comment

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

how funky would it be to popup a withProgress notification? That way at least get the progress animation

Copy link
Member Author

Choose a reason for hiding this comment

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

This is within a withProgress notification. Resetting the progress to 0 resets the animation.

// "complete" message has been sent, but does not report progress for this.
// Provide a suitable message in this case and reset the progress back to an
// indeterminate state (0) since we don't know how long it will take.
progress.report({
increment,
message:
progressData.step.text ??
`${progressData.step.percent}% complete`,
message: "Verifying signature and extracting...",
increment: -lastProgress,
});
lastProgress = progressData.step.percent;
return;
}
if (!progressData.step) {
return;
}
const increment = progressData.step.percent - lastProgress;
progress.report({
increment,
message:
progressData.step.text ?? `${progressData.step.percent}% complete`,
});
lastProgress = progressData.step.percent;
},
logger,
token
);

progress.report({
increment: 100 - lastProgress,
message: "Installation complete",
});
}
);

void vscode.window.showInformationMessage(`Successfully installed Swift ${version}`);

return true;
} catch (error) {
const errorMessage = (error as Error).message;
Expand All @@ -83,7 +88,7 @@ export async function installSwiftlyToolchainWithProgress(
return false;
}

logger?.error(`Failed to install Swift ${version}: ${error}`);
logger?.error(new Error(`Failed to install Swift ${version}`, { cause: error }));
void vscode.window.showErrorMessage(`Failed to install Swift ${version}: ${error}`);
return false;
}
Expand Down
30 changes: 20 additions & 10 deletions src/toolchain/swiftly.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,21 @@ const InUseVersionResult = z.object({
version: z.string(),
});

export interface SwiftlyProgressData {
step?: {
text?: string;
timestamp?: number;
percent?: number;
};
}
const SwiftlyProgressData = z.object({
complete: z.optional(
z.object({
success: z.boolean(),
})
),
step: z.optional(
z.object({
text: z.string(),
percent: z.number(),
})
),
});

export type SwiftlyProgressData = z.infer<typeof SwiftlyProgressData>;

export interface PostInstallValidationResult {
isValid: boolean;
Expand Down Expand Up @@ -510,10 +518,12 @@ export class Swiftly {
}

try {
const progressData = JSON.parse(line.trim()) as SwiftlyProgressData;
const progressData = SwiftlyProgressData.parse(JSON.parse(line));
progressCallback(progressData);
} catch (err) {
logger?.error(`Failed to parse progress line: ${err}`);
} catch (error) {
logger?.error(
new Error(`Failed to parse Swiftly progress: ${line}`, { cause: error })
);
}
});

Expand Down
4 changes: 2 additions & 2 deletions tsconfig-base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"rootDir": ".",
"outDir": "dist",

"lib": ["ES2021"],
"target": "ES2020",
"lib": ["ES2022"],
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to bump for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to get error causations. So, you can provide a cause (kind of like in Java) for your errors:

try {
   // Do something that will throw
} catch (error) {
   throw new Error("Something went wrong", { cause: error });
}

NodeJS will print the stack trace of both errors in this case so that we don't have to do this manually like what was in the existing code.

"target": "ES2022",
"module": "commonjs",

"strict": true,
Expand Down