From 6272b828beea56539bef41965ee86131bb896d7a Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 10:09:13 -0500 Subject: [PATCH 1/7] Notify build is preparing before progress starts The build progress notification would only appear once the progress values were shown in the output, i.e. [12/122]. Until progress values appear, show a "Preparing " notification. This only needs to apply when the `showBuildStatus` value is `notification` or `progress`, since `swiftStatus` already shows a message in the bar immediately. This patch also only sets the `showBuildStatus` setting on a Task if it is coming from a user defined task. Previously it was being set on synthetic tasks which were being cached, so when the user updated the `showBuildStatus` setting it wouldn't take effect until they restarted the extension. Issue: #1322 --- src/SwiftSnippets.ts | 1 - src/tasks/SwiftTaskProvider.ts | 15 +++++++++------ src/ui/SwiftBuildStatus.ts | 22 +++++++++++++++++++--- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/SwiftSnippets.ts b/src/SwiftSnippets.ts index 511283c40..a14e35efd 100644 --- a/src/SwiftSnippets.ts +++ b/src/SwiftSnippets.ts @@ -82,7 +82,6 @@ export async function debugSnippetWithOptions( presentationOptions: { reveal: vscode.TaskRevealKind.Always, }, - showBuildStatus: configuration.showBuildStatus, }, ctx.toolchain ); diff --git a/src/tasks/SwiftTaskProvider.ts b/src/tasks/SwiftTaskProvider.ts index 6f59e2433..1fb2b4b35 100644 --- a/src/tasks/SwiftTaskProvider.ts +++ b/src/tasks/SwiftTaskProvider.ts @@ -155,7 +155,6 @@ export function createBuildAllTask( reveal: getBuildRevealOption(), }, disableTaskQueue: true, - showBuildStatus: configuration.showBuildStatus, }, folderContext.workspaceContext.toolchain ); @@ -239,7 +238,6 @@ function createBuildTasks(product: Product, folderContext: FolderContext): vscod }, disableTaskQueue: true, dontTriggerTestDiscovery: true, - showBuildStatus: configuration.showBuildStatus, }, folderContext.workspaceContext.toolchain ); @@ -268,7 +266,6 @@ function createBuildTasks(product: Product, folderContext: FolderContext): vscod }, disableTaskQueue: true, dontTriggerTestDiscovery: true, - showBuildStatus: configuration.showBuildStatus, }, folderContext.workspaceContext.toolchain ); @@ -314,9 +311,15 @@ export function createSwiftTask( args: args, env: env, cwd: cwd, - showBuildStatus: config.showBuildStatus, - disableTaskQueue: config.disableTaskQueue, - dontTriggerTestDiscovery: config.dontTriggerTestDiscovery, + ...(config.showBuildStatus !== undefined + ? { showBuildStatus: config.showBuildStatus } + : {}), + ...(config.disableTaskQueue !== undefined + ? { disableTaskQueue: config.disableTaskQueue } + : {}), + ...(config.dontTriggerTestDiscovery !== undefined + ? { dontTriggerTestDiscovery: config.dontTriggerTestDiscovery } + : {}), }, config?.scope ?? vscode.TaskScope.Workspace, name, diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts index 2589977a9..6e5d3c68d 100644 --- a/src/ui/SwiftBuildStatus.ts +++ b/src/ui/SwiftBuildStatus.ts @@ -71,9 +71,10 @@ export class SwiftBuildStatus implements vscode.Disposable { disposables.forEach(d => d.dispose()); res(); }; + const state = { started: false }; disposables.push( execution.onDidWrite(data => { - if (this.parseEvents(task, data, update)) { + if (this.parseEvents(task, data, showBuildStatus, update, state)) { done(); } }), @@ -109,7 +110,9 @@ export class SwiftBuildStatus implements vscode.Disposable { private parseEvents( task: vscode.Task, data: string, - update: (message: string) => void + showBuildStatus: ShowBuildStatusOptions, + update: (message: string) => void, + state: { started: boolean } ): boolean { const name = new RunningTask(task).name; const sanitizedData = stripAnsi(data); @@ -124,14 +127,27 @@ export class SwiftBuildStatus implements vscode.Disposable { const progress = this.findBuildProgress(line); if (progress) { update(`${name} [${progress.completed}/${progress.total}]`); + state.started = true; return false; } if (this.checkIfFetching(line)) { // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); - update(`${name} fetching dependencies`); + update(`${name}: fetching dependencies`); + state.started = true; return false; } } + // If we've found nothing that matches a known state then put up a temporary + // message that we're preparing the build, as there is sometimes a delay before + // building starts while the build system is preparing, especially in large projects. + // The status bar has a message immediately, so only show this when using a + // notification to show progress. + if ( + !state.started && + (showBuildStatus === "notification" || showBuildStatus === "progress") + ) { + update(`Preparing ${name}`); + } return false; } From e4b8f8d310c705735850b56f867c5d987964fe79 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 10:20:09 -0500 Subject: [PATCH 2/7] Lint fix --- src/SwiftSnippets.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SwiftSnippets.ts b/src/SwiftSnippets.ts index a14e35efd..f7fa4cccb 100644 --- a/src/SwiftSnippets.ts +++ b/src/SwiftSnippets.ts @@ -19,7 +19,6 @@ import { createSwiftTask } from "./tasks/SwiftTaskProvider"; import { WorkspaceContext } from "./WorkspaceContext"; import { createSnippetConfiguration, debugLaunchConfig } from "./debugger/launch"; import { TaskOperation } from "./tasks/TaskQueue"; -import configuration from "./configuration"; /** * Set context key indicating whether current file is a Swift Snippet From dede60df821b428963b9dfb6cd2621719c179602 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 10:26:35 -0500 Subject: [PATCH 3/7] Tweak message --- src/ui/SwiftBuildStatus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts index 6e5d3c68d..32230d5d6 100644 --- a/src/ui/SwiftBuildStatus.ts +++ b/src/ui/SwiftBuildStatus.ts @@ -132,7 +132,7 @@ export class SwiftBuildStatus implements vscode.Disposable { } if (this.checkIfFetching(line)) { // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); - update(`${name}: fetching dependencies`); + update(`${name} fetching dependencies`); state.started = true; return false; } From b48dce36f6ad01177e7e97736a2d5042edeb6444 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 10:42:55 -0500 Subject: [PATCH 4/7] Tweak messages --- src/ui/SwiftBuildStatus.ts | 6 +++--- test/unit-tests/ui/SwiftBuildStatus.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts index 32230d5d6..c2014f872 100644 --- a/src/ui/SwiftBuildStatus.ts +++ b/src/ui/SwiftBuildStatus.ts @@ -126,13 +126,13 @@ export class SwiftBuildStatus implements vscode.Disposable { } const progress = this.findBuildProgress(line); if (progress) { - update(`${name} [${progress.completed}/${progress.total}]`); + update(`${name}: [${progress.completed}/${progress.total}]`); state.started = true; return false; } if (this.checkIfFetching(line)) { // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); - update(`${name} fetching dependencies`); + update(`${name}: Fetching Dependencies`); state.started = true; return false; } @@ -146,7 +146,7 @@ export class SwiftBuildStatus implements vscode.Disposable { !state.started && (showBuildStatus === "notification" || showBuildStatus === "progress") ) { - update(`Preparing ${name}`); + update(`${name}: Preparing...`); } return false; } diff --git a/test/unit-tests/ui/SwiftBuildStatus.test.ts b/test/unit-tests/ui/SwiftBuildStatus.test.ts index 4c66856af..5a17679ec 100644 --- a/test/unit-tests/ui/SwiftBuildStatus.test.ts +++ b/test/unit-tests/ui/SwiftBuildStatus.test.ts @@ -192,9 +192,9 @@ suite("SwiftBuildStatus Unit Test Suite", async function () { // Ignore old stuff expect(mockedProgress.report).to.not.have.been.calledWith({ - message: "My Task fetching dependencies", + message: "My Task: Fetching Dependencies", }); - expect(mockedProgress.report).to.not.have.been.calledWith({ message: "My Task [6/7]" }); + expect(mockedProgress.report).to.not.have.been.calledWith({ message: "My Task: [6/7]" }); }); test("Build complete", async () => { From 1758c8ed260a6ded393f60b9ecec90ee4cd3b4a5 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 10:58:37 -0500 Subject: [PATCH 5/7] Test tweaks --- test/unit-tests/ui/SwiftBuildStatus.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit-tests/ui/SwiftBuildStatus.test.ts b/test/unit-tests/ui/SwiftBuildStatus.test.ts index 5a17679ec..09adbae08 100644 --- a/test/unit-tests/ui/SwiftBuildStatus.test.ts +++ b/test/unit-tests/ui/SwiftBuildStatus.test.ts @@ -164,7 +164,7 @@ suite("SwiftBuildStatus Unit Test Suite", async function () { "Fetched https://github.com/apple/swift-testing.git from cache (0.77s)\n" ); - const expected = "My Task fetching dependencies"; + const expected = "My Task: Fetching Dependencies"; expect(mockedProgress.report).to.have.been.calledWith({ message: expected }); expect(mockedStatusItem.update).to.have.been.calledWith(mockedTaskExecution.task, expected); }); @@ -186,7 +186,7 @@ suite("SwiftBuildStatus Unit Test Suite", async function () { "[7/7] Applying MyCLI\n" ); - const expected = "My Task [7/7]"; + const expected = "My Task: [7/7]"; expect(mockedProgress.report).to.have.been.calledWith({ message: expected }); expect(mockedStatusItem.update).to.have.been.calledWith(mockedTaskExecution.task, expected); From 1189f1b896698e7acb8c56a1063ef5d4de06b750 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 12:55:21 -0500 Subject: [PATCH 6/7] Simplify parseEvents --- src/ui/SwiftBuildStatus.ts | 41 +++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts index c2014f872..d93c6c7b8 100644 --- a/src/ui/SwiftBuildStatus.ts +++ b/src/ui/SwiftBuildStatus.ts @@ -71,10 +71,25 @@ export class SwiftBuildStatus implements vscode.Disposable { disposables.forEach(d => d.dispose()); res(); }; - const state = { started: false }; + let started = false; disposables.push( execution.onDidWrite(data => { - if (this.parseEvents(task, data, showBuildStatus, update, state)) { + const name = new RunningTask(task).name; + + // If we've found nothing that matches a known state then put up a temporary + // message that we're preparing the build, as there is sometimes a delay before + // building starts while the build system is preparing, especially in large projects. + // The status bar has a message immediately, so only show this when using a + // notification to show progress. + if ( + !started && + (showBuildStatus === "notification" || showBuildStatus === "progress") + ) { + update(`${name}: Preparing...`); + } + + started = true; + if (this.parseEvents(name, data, update)) { done(); } }), @@ -107,14 +122,7 @@ export class SwiftBuildStatus implements vscode.Disposable { * @param data * @returns true if done, false otherwise */ - private parseEvents( - task: vscode.Task, - data: string, - showBuildStatus: ShowBuildStatusOptions, - update: (message: string) => void, - state: { started: boolean } - ): boolean { - const name = new RunningTask(task).name; + private parseEvents(name: string, data: string, update: (message: string) => void): boolean { const sanitizedData = stripAnsi(data); // We'll process data one line at a time, in reverse order // since the latest interesting message is all we need to @@ -127,27 +135,14 @@ export class SwiftBuildStatus implements vscode.Disposable { const progress = this.findBuildProgress(line); if (progress) { update(`${name}: [${progress.completed}/${progress.total}]`); - state.started = true; return false; } if (this.checkIfFetching(line)) { // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); update(`${name}: Fetching Dependencies`); - state.started = true; return false; } } - // If we've found nothing that matches a known state then put up a temporary - // message that we're preparing the build, as there is sometimes a delay before - // building starts while the build system is preparing, especially in large projects. - // The status bar has a message immediately, so only show this when using a - // notification to show progress. - if ( - !state.started && - (showBuildStatus === "notification" || showBuildStatus === "progress") - ) { - update(`${name}: Preparing...`); - } return false; } From a15518fb4825d99d9b716781b22513830a3c38c8 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 21 Jan 2025 13:20:15 -0500 Subject: [PATCH 7/7] Refactor so no preparing is shown if build completes immediately --- src/ui/SwiftBuildStatus.ts | 99 +++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts index d93c6c7b8..db1681b53 100644 --- a/src/ui/SwiftBuildStatus.ts +++ b/src/ui/SwiftBuildStatus.ts @@ -71,28 +71,14 @@ export class SwiftBuildStatus implements vscode.Disposable { disposables.forEach(d => d.dispose()); res(); }; - let started = false; disposables.push( - execution.onDidWrite(data => { - const name = new RunningTask(task).name; - - // If we've found nothing that matches a known state then put up a temporary - // message that we're preparing the build, as there is sometimes a delay before - // building starts while the build system is preparing, especially in large projects. - // The status bar has a message immediately, so only show this when using a - // notification to show progress. - if ( - !started && - (showBuildStatus === "notification" || showBuildStatus === "progress") - ) { - update(`${name}: Preparing...`); - } - - started = true; - if (this.parseEvents(name, data, update)) { - done(); - } - }), + this.outputParser( + new RunningTask(task).name, + execution, + showBuildStatus, + update, + done + ), execution.onDidClose(done), vscode.tasks.onDidEndTask(e => { if (e.execution.task === task) { @@ -118,32 +104,57 @@ export class SwiftBuildStatus implements vscode.Disposable { } } - /** - * @param data - * @returns true if done, false otherwise - */ - private parseEvents(name: string, data: string, update: (message: string) => void): boolean { - const sanitizedData = stripAnsi(data); - // We'll process data one line at a time, in reverse order - // since the latest interesting message is all we need to - // be concerned with - const lines = sanitizedData.split(/\r\n|\n|\r/gm).reverse(); - for (const line of lines) { - if (checkIfBuildComplete(line)) { - return true; + private outputParser( + name: string, + execution: SwiftExecution, + showBuildStatus: ShowBuildStatusOptions, + update: (message: string) => void, + done: () => void + ): vscode.Disposable { + let started = false; + + const parseEvents = (data: string) => { + const sanitizedData = stripAnsi(data); + // We'll process data one line at a time, in reverse order + // since the latest interesting message is all we need to + // be concerned with + const lines = sanitizedData.split(/\r\n|\n|\r/gm).reverse(); + for (const line of lines) { + if (checkIfBuildComplete(line)) { + return true; + } + const progress = this.findBuildProgress(line); + if (progress) { + update(`${name}: [${progress.completed}/${progress.total}]`); + started = true; + return false; + } + if (this.checkIfFetching(line)) { + // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); + update(`${name}: Fetching Dependencies`); + started = true; + return false; + } } - const progress = this.findBuildProgress(line); - if (progress) { - update(`${name}: [${progress.completed}/${progress.total}]`); - return false; + // If we've found nothing that matches a known state then put up a temporary + // message that we're preparing the build, as there is sometimes a delay before + // building starts while the build system is preparing, especially in large projects. + // The status bar has a message immediately, so only show this when using a + // notification to show progress. + if ( + !started && + (showBuildStatus === "notification" || showBuildStatus === "progress") + ) { + update(`${name}: Preparing...`); } - if (this.checkIfFetching(line)) { - // this.statusItem.update(task, `Fetching dependencies "${task.name}"`); - update(`${name}: Fetching Dependencies`); - return false; + return false; + }; + + return execution.onDidWrite(data => { + if (parseEvents(data)) { + done(); } - } - return false; + }); } private checkIfFetching(line: string): boolean {