Skip to content

Commit d88a5c8

Browse files
plemarquandplemarquand
authored
Notify build is preparing before progress starts (#1323)
* 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 <task>" 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
1 parent 77f683b commit d88a5c8

File tree

4 files changed

+68
-45
lines changed

4 files changed

+68
-45
lines changed

Diff for: ‎src/SwiftSnippets.ts

-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { createSwiftTask } from "./tasks/SwiftTaskProvider";
1919
import { WorkspaceContext } from "./WorkspaceContext";
2020
import { createSnippetConfiguration, debugLaunchConfig } from "./debugger/launch";
2121
import { TaskOperation } from "./tasks/TaskQueue";
22-
import configuration from "./configuration";
2322

2423
/**
2524
* Set context key indicating whether current file is a Swift Snippet
@@ -82,7 +81,6 @@ export async function debugSnippetWithOptions(
8281
presentationOptions: {
8382
reveal: vscode.TaskRevealKind.Always,
8483
},
85-
showBuildStatus: configuration.showBuildStatus,
8684
},
8785
ctx.toolchain
8886
);

Diff for: ‎src/tasks/SwiftTaskProvider.ts

+9-6
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ export function createBuildAllTask(
155155
reveal: getBuildRevealOption(),
156156
},
157157
disableTaskQueue: true,
158-
showBuildStatus: configuration.showBuildStatus,
159158
},
160159
folderContext.workspaceContext.toolchain
161160
);
@@ -239,7 +238,6 @@ function createBuildTasks(product: Product, folderContext: FolderContext): vscod
239238
},
240239
disableTaskQueue: true,
241240
dontTriggerTestDiscovery: true,
242-
showBuildStatus: configuration.showBuildStatus,
243241
},
244242
folderContext.workspaceContext.toolchain
245243
);
@@ -268,7 +266,6 @@ function createBuildTasks(product: Product, folderContext: FolderContext): vscod
268266
},
269267
disableTaskQueue: true,
270268
dontTriggerTestDiscovery: true,
271-
showBuildStatus: configuration.showBuildStatus,
272269
},
273270
folderContext.workspaceContext.toolchain
274271
);
@@ -314,9 +311,15 @@ export function createSwiftTask(
314311
args: args,
315312
env: env,
316313
cwd: cwd,
317-
showBuildStatus: config.showBuildStatus,
318-
disableTaskQueue: config.disableTaskQueue,
319-
dontTriggerTestDiscovery: config.dontTriggerTestDiscovery,
314+
...(config.showBuildStatus !== undefined
315+
? { showBuildStatus: config.showBuildStatus }
316+
: {}),
317+
...(config.disableTaskQueue !== undefined
318+
? { disableTaskQueue: config.disableTaskQueue }
319+
: {}),
320+
...(config.dontTriggerTestDiscovery !== undefined
321+
? { dontTriggerTestDiscovery: config.dontTriggerTestDiscovery }
322+
: {}),
320323
},
321324
config?.scope ?? vscode.TaskScope.Workspace,
322325
name,

Diff for: ‎src/ui/SwiftBuildStatus.ts

+55-33
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,13 @@ export class SwiftBuildStatus implements vscode.Disposable {
7272
res();
7373
};
7474
disposables.push(
75-
execution.onDidWrite(data => {
76-
if (this.parseEvents(task, data, update)) {
77-
done();
78-
}
79-
}),
75+
this.outputParser(
76+
new RunningTask(task).name,
77+
execution,
78+
showBuildStatus,
79+
update,
80+
done
81+
),
8082
execution.onDidClose(done),
8183
vscode.tasks.onDidEndTask(e => {
8284
if (e.execution.task === task) {
@@ -102,37 +104,57 @@ export class SwiftBuildStatus implements vscode.Disposable {
102104
}
103105
}
104106

105-
/**
106-
* @param data
107-
* @returns true if done, false otherwise
108-
*/
109-
private parseEvents(
110-
task: vscode.Task,
111-
data: string,
112-
update: (message: string) => void
113-
): boolean {
114-
const name = new RunningTask(task).name;
115-
const sanitizedData = stripAnsi(data);
116-
// We'll process data one line at a time, in reverse order
117-
// since the latest interesting message is all we need to
118-
// be concerned with
119-
const lines = sanitizedData.split(/\r\n|\n|\r/gm).reverse();
120-
for (const line of lines) {
121-
if (checkIfBuildComplete(line)) {
122-
return true;
107+
private outputParser(
108+
name: string,
109+
execution: SwiftExecution,
110+
showBuildStatus: ShowBuildStatusOptions,
111+
update: (message: string) => void,
112+
done: () => void
113+
): vscode.Disposable {
114+
let started = false;
115+
116+
const parseEvents = (data: string) => {
117+
const sanitizedData = stripAnsi(data);
118+
// We'll process data one line at a time, in reverse order
119+
// since the latest interesting message is all we need to
120+
// be concerned with
121+
const lines = sanitizedData.split(/\r\n|\n|\r/gm).reverse();
122+
for (const line of lines) {
123+
if (checkIfBuildComplete(line)) {
124+
return true;
125+
}
126+
const progress = this.findBuildProgress(line);
127+
if (progress) {
128+
update(`${name}: [${progress.completed}/${progress.total}]`);
129+
started = true;
130+
return false;
131+
}
132+
if (this.checkIfFetching(line)) {
133+
// this.statusItem.update(task, `Fetching dependencies "${task.name}"`);
134+
update(`${name}: Fetching Dependencies`);
135+
started = true;
136+
return false;
137+
}
123138
}
124-
const progress = this.findBuildProgress(line);
125-
if (progress) {
126-
update(`${name} [${progress.completed}/${progress.total}]`);
127-
return false;
139+
// If we've found nothing that matches a known state then put up a temporary
140+
// message that we're preparing the build, as there is sometimes a delay before
141+
// building starts while the build system is preparing, especially in large projects.
142+
// The status bar has a message immediately, so only show this when using a
143+
// notification to show progress.
144+
if (
145+
!started &&
146+
(showBuildStatus === "notification" || showBuildStatus === "progress")
147+
) {
148+
update(`${name}: Preparing...`);
128149
}
129-
if (this.checkIfFetching(line)) {
130-
// this.statusItem.update(task, `Fetching dependencies "${task.name}"`);
131-
update(`${name} fetching dependencies`);
132-
return false;
150+
return false;
151+
};
152+
153+
return execution.onDidWrite(data => {
154+
if (parseEvents(data)) {
155+
done();
133156
}
134-
}
135-
return false;
157+
});
136158
}
137159

138160
private checkIfFetching(line: string): boolean {

Diff for: ‎test/unit-tests/ui/SwiftBuildStatus.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ suite("SwiftBuildStatus Unit Test Suite", async function () {
164164
"Fetched https://github.com/apple/swift-testing.git from cache (0.77s)\n"
165165
);
166166

167-
const expected = "My Task fetching dependencies";
167+
const expected = "My Task: Fetching Dependencies";
168168
expect(mockedProgress.report).to.have.been.calledWith({ message: expected });
169169
expect(mockedStatusItem.update).to.have.been.calledWith(mockedTaskExecution.task, expected);
170170
});
@@ -186,15 +186,15 @@ suite("SwiftBuildStatus Unit Test Suite", async function () {
186186
"[7/7] Applying MyCLI\n"
187187
);
188188

189-
const expected = "My Task [7/7]";
189+
const expected = "My Task: [7/7]";
190190
expect(mockedProgress.report).to.have.been.calledWith({ message: expected });
191191
expect(mockedStatusItem.update).to.have.been.calledWith(mockedTaskExecution.task, expected);
192192

193193
// Ignore old stuff
194194
expect(mockedProgress.report).to.not.have.been.calledWith({
195-
message: "My Task fetching dependencies",
195+
message: "My Task: Fetching Dependencies",
196196
});
197-
expect(mockedProgress.report).to.not.have.been.calledWith({ message: "My Task [6/7]" });
197+
expect(mockedProgress.report).to.not.have.been.calledWith({ message: "My Task: [6/7]" });
198198
});
199199

200200
test("Build complete", async () => {

0 commit comments

Comments
 (0)