Skip to content

Commit 396f876

Browse files
committed
terminal: apply global selection event payload
Avoid re-querying the environment API while handling a global selection change, since it can still expose the previous selection. Invalidate pending initialization and update shell-startup variables from the authoritative event payload instead.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5051951 commit 396f876

2 files changed

Lines changed: 31 additions & 26 deletions

File tree

‎src/features/terminal/shellStartupActivationVariablesManager.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
5656
}
5757
if (!e.uri) {
5858
if (!getWorkspaceFolders()?.length) {
59-
await this.initializeInternal();
59+
++this.globalRefreshGeneration;
60+
this.updateGlobalEnvironment(e.new);
6061
}
6162
return;
6263
}
@@ -103,18 +104,20 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
103104
if (generation !== this.globalRefreshGeneration || getWorkspaceFolders()?.length) {
104105
return;
105106
}
106-
await Promise.all(
107-
this.shellEnvsProviders.map(async (provider) => {
108-
if (env) {
109-
provider.updateEnvVariables(this.envCollection, env);
110-
} else {
111-
provider.removeEnvVariables(this.envCollection);
112-
}
113-
}),
114-
);
107+
this.updateGlobalEnvironment(env);
115108
}
116109
}
117110

111+
private updateGlobalEnvironment(environment: DidChangeEnvironmentEventArgs['new']): void {
112+
this.shellEnvsProviders.forEach((provider) => {
113+
if (environment) {
114+
provider.updateEnvVariables(this.envCollection, environment);
115+
} else {
116+
provider.removeEnvVariables(this.envCollection);
117+
}
118+
});
119+
}
120+
118121
public async initialize(): Promise<void> {
119122
const autoActType = getAutoActivationType();
120123
if (autoActType === ACT_TYPE_SHELL) {

‎src/test/features/terminal/shellStartupActivationVariablesManager.unit.test.ts‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,30 +139,32 @@ suite('ShellStartupActivationVariablesManager', () => {
139139
test('refreshes global startup variables after a global selection changes', async () => {
140140
const newEnvironment = makeEnvironment('global-venv-b', 'ms-python.python:venv');
141141
await changeListener!({ uri: undefined, new: folderEnvironment, old: undefined });
142-
getEnvironmentStub.resolves(newEnvironment);
143-
144142
await changeListener!({ uri: undefined, new: newEnvironment, old: folderEnvironment });
145143

146-
sinon.assert.calledTwice(getEnvironmentStub);
147-
sinon.assert.alwaysCalledWithExactly(getEnvironmentStub, undefined);
144+
sinon.assert.notCalled(getEnvironmentStub);
148145
assert.deepStrictEqual(provider.updated, [folderEnvironment, newEnvironment]);
149146
assert.deepStrictEqual(provider.updatedCollections, [envCollection, envCollection]);
150147
});
151148

152-
test('does not let an older global refresh overwrite a newer selection', async () => {
149+
test('uses the global change payload while the API still returns the previous selection', async () => {
153150
const newEnvironment = makeEnvironment('global-venv-b', 'ms-python.python:venv');
154-
const olderRefresh = createDeferred<PythonEnvironment | undefined>();
155-
const newerRefresh = createDeferred<PythonEnvironment | undefined>();
156-
getEnvironmentStub.onFirstCall().returns(olderRefresh.promise);
157-
getEnvironmentStub.onSecondCall().returns(newerRefresh.promise);
151+
getEnvironmentStub.resolves(folderEnvironment);
158152

159-
const firstChange = changeListener!({ uri: undefined, new: folderEnvironment, old: undefined });
160-
const secondChange = changeListener!({ uri: undefined, new: newEnvironment, old: folderEnvironment });
153+
await changeListener!({ uri: undefined, new: newEnvironment, old: folderEnvironment });
161154

162-
newerRefresh.resolve(newEnvironment);
163-
await secondChange;
164-
olderRefresh.resolve(folderEnvironment);
165-
await firstChange;
155+
sinon.assert.notCalled(getEnvironmentStub);
156+
assert.deepStrictEqual(provider.updated, [newEnvironment]);
157+
});
158+
159+
test('does not let pending global initialization overwrite a newer selection', async () => {
160+
const newEnvironment = makeEnvironment('global-venv-b', 'ms-python.python:venv');
161+
const pendingInitialization = createDeferred<PythonEnvironment | undefined>();
162+
getEnvironmentStub.returns(pendingInitialization.promise);
163+
164+
const initialization = manager.initialize();
165+
await changeListener!({ uri: undefined, new: newEnvironment, old: folderEnvironment });
166+
pendingInitialization.resolve(folderEnvironment);
167+
await initialization;
166168

167169
assert.deepStrictEqual(provider.updated, [newEnvironment]);
168170
assert.deepStrictEqual(provider.updatedCollections, [envCollection]);
@@ -189,7 +191,7 @@ suite('ShellStartupActivationVariablesManager', () => {
189191

190192
await changeListener!({ uri: undefined, new: undefined, old: folderEnvironment });
191193

192-
sinon.assert.calledOnceWithExactly(getEnvironmentStub, undefined);
194+
sinon.assert.notCalled(getEnvironmentStub);
193195
assert.deepStrictEqual(provider.removedCollections, [envCollection]);
194196
assert.strictEqual(provider.updated.length, 0);
195197
});

0 commit comments

Comments
 (0)