Skip to content

Commit 6322877

Browse files
authored
Give current state for new FolderOperation listeners (#1945)
* Give current state for new FolderOperation listeners - Iterate through all current folders and call "add" - Call "focus" if there is a currently focussed folder - Make sure the listener callbacks throwing don't prevent remaining callbacks Issue: #1944 * Add CHANGELOG entry * Fix failing test
1 parent 608182d commit 6322877

File tree

3 files changed

+76
-38
lines changed

3 files changed

+76
-38
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Fixed
66

77
- Suggest "Open Documentation" when toolchain not found ([#1939](https://github.com/swiftlang/vscode-swift/pull/1939))
8+
- Make sure all folder operation listeners get past folder add events ([#1945](https://github.com/swiftlang/vscode-swift/pull/1945))
89

910
## 2.14.0 - 2025-11-11
1011

src/WorkspaceContext.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,14 @@ export class WorkspaceContext implements vscode.Disposable {
380380
*/
381381
async fireEvent(folder: FolderContext | null, operation: FolderOperation) {
382382
for (const observer of this.observers) {
383-
await observer({ folder, operation, workspace: this });
383+
try {
384+
await observer({ folder, operation, workspace: this });
385+
} catch (error) {
386+
// Make sure one observer does not stop all others from being called
387+
this.logger.error(
388+
`Folder operation "${operation}" event observer failed for ${folder?.folder.fsPath}: ${error}`
389+
);
390+
}
384391
}
385392
}
386393

@@ -525,6 +532,20 @@ export class WorkspaceContext implements vscode.Disposable {
525532

526533
onDidChangeFolders(listener: (event: FolderEvent) => unknown): vscode.Disposable {
527534
this.observers.add(listener);
535+
536+
// https://github.com/swiftlang/vscode-swift/issues/1944
537+
// make sure no FolderOperation are missed by fast activation
538+
for (const folder of this.folders) {
539+
listener({ folder, operation: FolderOperation.add, workspace: this });
540+
}
541+
if (this.currentFolder) {
542+
listener({
543+
folder: this.currentFolder,
544+
operation: FolderOperation.focus,
545+
workspace: this,
546+
});
547+
}
548+
528549
return { dispose: () => this.observers.delete(listener) };
529550
}
530551

test/integration-tests/WorkspaceContext.test.ts

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// SPDX-License-Identifier: Apache-2.0
1212
//
1313
//===----------------------------------------------------------------------===//
14-
import * as assert from "assert";
14+
import { expect } from "chai";
1515
import { afterEach } from "mocha";
1616
import * as vscode from "vscode";
1717

@@ -22,7 +22,7 @@ import { createBuildAllTask } from "@src/tasks/SwiftTaskProvider";
2222
import { resolveScope } from "@src/utilities/tasks";
2323
import { Version } from "@src/utilities/version";
2424

25-
import { testAssetUri } from "../fixtures";
25+
import { testAssetPath, testAssetUri } from "../fixtures";
2626
import { tag } from "../tags";
2727
import { assertContains } from "./testexplorer/utilities";
2828
import {
@@ -32,14 +32,11 @@ import {
3232
} from "./utilities/testutilities";
3333

3434
function assertContainsArg(execution: SwiftExecution, arg: string) {
35-
assert(execution?.args.find(a => a === arg));
35+
expect(execution?.args.find(a => a === arg)).to.not.be.undefined;
3636
}
3737

3838
function assertNotContainsArg(execution: SwiftExecution, arg: string) {
39-
assert.equal(
40-
execution?.args.find(a => a.includes(arg)),
41-
undefined
42-
);
39+
expect(execution?.args.find(a => a.includes(arg))).to.be.undefined;
4340
}
4441

4542
tag("medium").suite("WorkspaceContext Test Suite", () => {
@@ -57,7 +54,7 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
5754

5855
test("Add", async () => {
5956
let observer: vscode.Disposable | undefined;
60-
const recordedFolders: {
57+
let recordedFolders: {
6158
folder: FolderContext | null;
6259
operation: FolderOperation;
6360
}[] = [];
@@ -67,10 +64,28 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
6764
recordedFolders.push(changedFolderRecord);
6865
});
6966

67+
// https://github.com/swiftlang/vscode-swift/issues/1944
68+
// make sure get existing folder(s)
69+
const addedFolders = recordedFolders.filter(
70+
({ operation }) => operation === FolderOperation.add
71+
);
72+
let addedCount = addedFolders.length;
73+
expect(
74+
addedCount,
75+
`Expected at least one add folder operation, instead got folders: ${addedFolders.map(folder => folder.folder?.name)}`
76+
).to.be.greaterThanOrEqual(1);
77+
console.log(addedFolders.map(folder => folder.folder?.name));
78+
expect(
79+
addedFolders.find(
80+
folder => folder?.folder?.folder.fsPath === testAssetPath("defaultPackage")
81+
)
82+
).to.not.be.undefined;
83+
7084
const workspaceFolder = getRootWorkspaceFolder();
7185

72-
assert.ok(workspaceFolder, "No workspace folders found in workspace");
86+
expect(workspaceFolder).to.not.be.undefined;
7387

88+
recordedFolders = [];
7489
await workspaceContext.addPackageFolder(testAssetUri("package2"), workspaceFolder);
7590

7691
const foldersNamePromises = recordedFolders
@@ -79,14 +94,13 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
7994
const foldersNames = await Promise.all(foldersNamePromises);
8095
assertContains(foldersNames, "package2");
8196

82-
const addedCount = recordedFolders.filter(
97+
addedCount = recordedFolders.filter(
8398
({ operation }) => operation === FolderOperation.add
8499
).length;
85-
assert.strictEqual(
100+
expect(
86101
addedCount,
87-
1,
88102
`Expected only one add folder operation, instead got folders: ${recordedFolders.map(folder => folder.folder?.name)}`
89-
);
103+
).to.equal(1);
90104
} finally {
91105
observer?.dispose();
92106
}
@@ -112,66 +126,66 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
112126
const folder = workspaceContext.folders.find(
113127
f => f.folder.fsPath === packageFolder.fsPath
114128
);
115-
assert(folder);
129+
expect(folder).to.not.be.undefined;
116130
resetSettings = await updateSettings({
117131
"swift.diagnosticsStyle": "",
118132
});
119-
const buildAllTask = await createBuildAllTask(folder);
133+
const buildAllTask = await createBuildAllTask(folder!);
120134
const execution = buildAllTask.execution;
121-
assert.strictEqual(buildAllTask.definition.type, "swift");
122-
assert.strictEqual(buildAllTask.name, "Build All (defaultPackage)");
135+
expect(buildAllTask.definition.type).to.equal("swift");
136+
expect(buildAllTask.name).to.equal("Build All (defaultPackage)");
123137
assertContainsArg(execution, "build");
124138
assertContainsArg(execution, "--build-tests");
125-
assert.strictEqual(buildAllTask.scope, resolveScope(folder.workspaceFolder));
139+
expect(buildAllTask.scope).to.equal(resolveScope(folder!.workspaceFolder));
126140
});
127141

128142
test('"default" diagnosticsStyle', async () => {
129143
const folder = workspaceContext.folders.find(
130144
f => f.folder.fsPath === packageFolder.fsPath
131145
);
132-
assert(folder);
146+
expect(folder).to.not.be.undefined;
133147
resetSettings = await updateSettings({
134148
"swift.diagnosticsStyle": "default",
135149
});
136-
const buildAllTask = await createBuildAllTask(folder);
150+
const buildAllTask = await createBuildAllTask(folder!);
137151
const execution = buildAllTask.execution;
138-
assert.strictEqual(buildAllTask.definition.type, "swift");
139-
assert.strictEqual(buildAllTask.name, "Build All (defaultPackage)");
152+
expect(buildAllTask.definition.type).to.equal("swift");
153+
expect(buildAllTask.name).to.equal("Build All (defaultPackage)");
140154
assertContainsArg(execution, "build");
141155
assertContainsArg(execution, "--build-tests");
142156
assertNotContainsArg(execution, "-diagnostic-style");
143-
assert.strictEqual(buildAllTask.scope, resolveScope(folder.workspaceFolder));
157+
expect(buildAllTask.scope).to.equal(resolveScope(folder!.workspaceFolder));
144158
});
145159

146160
test('"swift" diagnosticsStyle', async () => {
147161
const folder = workspaceContext.folders.find(
148162
f => f.folder.fsPath === packageFolder.fsPath
149163
);
150-
assert(folder);
164+
expect(folder).to.not.be.undefined;
151165
resetSettings = await updateSettings({
152166
"swift.diagnosticsStyle": "swift",
153167
});
154-
const buildAllTask = await createBuildAllTask(folder);
168+
const buildAllTask = await createBuildAllTask(folder!);
155169
const execution = buildAllTask.execution;
156-
assert.strictEqual(buildAllTask.definition.type, "swift");
157-
assert.strictEqual(buildAllTask.name, "Build All (defaultPackage)");
170+
expect(buildAllTask.definition.type).to.equal("swift");
171+
expect(buildAllTask.name).to.equal("Build All (defaultPackage)");
158172
assertContainsArg(execution, "build");
159173
assertContainsArg(execution, "--build-tests");
160174
assertContainsArg(execution, "-Xswiftc");
161175
assertContainsArg(execution, "-diagnostic-style=swift");
162-
assert.strictEqual(buildAllTask.scope, resolveScope(folder.workspaceFolder));
176+
expect(buildAllTask.scope).to.equal(resolveScope(folder!.workspaceFolder));
163177
});
164178

165179
test("Build Settings", async () => {
166180
const folder = workspaceContext.folders.find(
167181
f => f.folder.fsPath === packageFolder.fsPath
168182
);
169-
assert(folder);
183+
expect(folder).to.not.be.undefined;
170184
resetSettings = await updateSettings({
171185
"swift.diagnosticsStyle": "",
172186
"swift.buildArguments": ["--sanitize=thread"],
173187
});
174-
const buildAllTask = await createBuildAllTask(folder);
188+
const buildAllTask = await createBuildAllTask(folder!);
175189
const execution = buildAllTask.execution as SwiftExecution;
176190
assertContainsArg(execution, "--sanitize=thread");
177191
});
@@ -180,12 +194,12 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
180194
const folder = workspaceContext.folders.find(
181195
f => f.folder.fsPath === packageFolder.fsPath
182196
);
183-
assert(folder);
197+
expect(folder).to.not.be.undefined;
184198
resetSettings = await updateSettings({
185199
"swift.diagnosticsStyle": "",
186200
"swift.packageArguments": ["--replace-scm-with-registry"],
187201
});
188-
const buildAllTask = await createBuildAllTask(folder);
202+
const buildAllTask = await createBuildAllTask(folder!);
189203
const execution = buildAllTask.execution as SwiftExecution;
190204
assertContainsArg(execution, "--replace-scm-with-registry");
191205
});
@@ -202,16 +216,18 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
202216
// This is only supported in swift versions >=5.8.0
203217
const swiftVersion = workspaceContext.globalToolchain.swiftVersion;
204218
if (swiftVersion.isLessThan(new Version(5, 8, 0))) {
205-
assert.deepEqual(await workspaceContext.globalToolchain.getProjectTemplates(), []);
219+
await expect(
220+
workspaceContext.globalToolchain.getProjectTemplates()
221+
).to.eventually.deep.equal([]);
206222
return;
207223
}
208224
// The output of `swift package init --help` will probably change at some point.
209225
// Just make sure that the most complex portions of the output are parsed correctly.
210226
const projectTemplates = await workspaceContext.globalToolchain.getProjectTemplates();
211227
// Contains multi-line description
212228
const toolTemplate = projectTemplates.find(template => template.id === "tool");
213-
assert(toolTemplate);
214-
assert.deepEqual(toolTemplate, {
229+
expect(toolTemplate).to.not.be.undefined;
230+
expect(toolTemplate).to.deep.equal({
215231
id: "tool",
216232
name: "Tool",
217233
description:
@@ -225,8 +241,8 @@ tag("medium").suite("WorkspaceContext Test Suite", () => {
225241
const buildToolPluginTemplate = projectTemplates.find(
226242
t => t.id === "build-tool-plugin"
227243
);
228-
assert(buildToolPluginTemplate);
229-
assert.deepEqual(buildToolPluginTemplate, {
244+
expect(buildToolPluginTemplate).to.not.be.undefined;
245+
expect(buildToolPluginTemplate).to.deep.equal({
230246
id: "build-tool-plugin",
231247
name: "Build Tool Plugin",
232248
description: "A package that vends a build tool plugin.",

0 commit comments

Comments
 (0)