Skip to content

Commit c213491

Browse files
author
Kartik Raj
authored
Apply environment variables after shell initialization scripts are run in pythonTerminalEnvVarActivation experiment (#21290)
For #11039 #20822 Closes #21297 Update proposed APIs to be used in Terminal activation experiment.
1 parent 72f7ef8 commit c213491

File tree

6 files changed

+141
-70
lines changed

6 files changed

+141
-70
lines changed

‎package-lock.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
"testObserver",
2525
"quickPickItemTooltip",
2626
"envCollectionWorkspace",
27-
"saveEditor"
27+
"saveEditor",
28+
"envCollectionOptions"
2829
],
2930
"author": {
3031
"name": "Microsoft Corporation"
@@ -45,7 +46,7 @@
4546
"theme": "dark"
4647
},
4748
"engines": {
48-
"vscode": "^1.78.0-20230421"
49+
"vscode": "^1.79.0-20230526"
4950
},
5051
"keywords": [
5152
"python",

‎src/client/interpreter/activation/terminalEnvVarCollectionService.ts

+26-12
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@
33

44
import * as path from 'path';
55
import { inject, injectable } from 'inversify';
6-
import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
6+
import {
7+
ProgressOptions,
8+
ProgressLocation,
9+
MarkdownString,
10+
WorkspaceFolder,
11+
EnvironmentVariableCollection,
12+
EnvironmentVariableScope,
13+
} from 'vscode';
714
import { pathExists } from 'fs-extra';
815
import { IExtensionActivationService } from '../../activation/types';
916
import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
@@ -108,6 +115,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
108115
undefined,
109116
shell,
110117
);
118+
const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder);
111119
if (!env) {
112120
const shellType = identifyShellFromShellPath(shell);
113121
const defaultShell = defaultShells[this.platform.osType];
@@ -117,7 +125,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
117125
await this._applyCollection(resource, defaultShell?.shell);
118126
return;
119127
}
120-
this.context.environmentVariableCollection.clear({ workspaceFolder });
128+
envVarCollection.clear();
121129
this.previousEnvVars = _normCaseKeys(process.env);
122130
return;
123131
}
@@ -129,25 +137,32 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
129137
if (prevValue !== value) {
130138
if (value !== undefined) {
131139
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
132-
this.context.environmentVariableCollection.replace(key, value, { workspaceFolder });
140+
envVarCollection.replace(key, value, { applyAtShellIntegration: true });
133141
} else {
134142
traceVerbose(`Clearing environment variable ${key} from collection`);
135-
this.context.environmentVariableCollection.delete(key, { workspaceFolder });
143+
envVarCollection.delete(key);
136144
}
137145
}
138146
});
139147
Object.keys(previousEnv).forEach((key) => {
140148
// If the previous env var is not in the current env, clear it from collection.
141149
if (!(key in env)) {
142150
traceVerbose(`Clearing environment variable ${key} from collection`);
143-
this.context.environmentVariableCollection.delete(key, { workspaceFolder });
151+
envVarCollection.delete(key);
144152
}
145153
});
146154
const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath);
147155
const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``);
148-
this.context.environmentVariableCollection.setDescription(description, {
149-
workspaceFolder,
150-
});
156+
envVarCollection.description = description;
157+
}
158+
159+
private getEnvironmentVariableCollection(workspaceFolder?: WorkspaceFolder) {
160+
const envVarCollection = this.context.environmentVariableCollection as EnvironmentVariableCollection & {
161+
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
162+
};
163+
return workspaceFolder
164+
? envVarCollection.getScopedEnvironmentVariableCollection({ workspaceFolder })
165+
: envVarCollection;
151166
}
152167

153168
private async handleMicroVenv(resource: Resource) {
@@ -156,12 +171,11 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
156171
if (interpreter?.envType === EnvironmentType.Venv) {
157172
const activatePath = path.join(path.dirname(interpreter.path), 'activate');
158173
if (!(await pathExists(activatePath))) {
159-
this.context.environmentVariableCollection.replace(
174+
const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder);
175+
envVarCollection.replace(
160176
'PATH',
161177
`${path.dirname(interpreter.path)}${path.delimiter}${process.env.Path}`,
162-
{
163-
workspaceFolder,
164-
},
178+
{ applyAtShellIntegration: true },
165179
);
166180
return;
167181
}

‎src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

+35-31
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ import * as sinon from 'sinon';
77
import { assert, expect } from 'chai';
88
import { cloneDeep } from 'lodash';
99
import { mock, instance, when, anything, verify, reset } from 'ts-mockito';
10-
import { EnvironmentVariableCollection, ProgressLocation, Uri, WorkspaceFolder } from 'vscode';
10+
import {
11+
EnvironmentVariableCollection,
12+
EnvironmentVariableScope,
13+
ProgressLocation,
14+
Uri,
15+
WorkspaceFolder,
16+
} from 'vscode';
1117
import {
1218
IApplicationShell,
1319
IApplicationEnvironment,
@@ -39,7 +45,10 @@ suite('Terminal Environment Variable Collection Service', () => {
3945
let context: IExtensionContext;
4046
let shell: IApplicationShell;
4147
let experimentService: IExperimentService;
42-
let collection: EnvironmentVariableCollection;
48+
let collection: EnvironmentVariableCollection & {
49+
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
50+
};
51+
let scopedCollection: EnvironmentVariableCollection;
4352
let applicationEnvironment: IApplicationEnvironment;
4453
let environmentActivationService: IEnvironmentActivationService;
4554
let workspaceService: IWorkspaceService;
@@ -62,7 +71,13 @@ suite('Terminal Environment Variable Collection Service', () => {
6271
interpreterService = mock<IInterpreterService>();
6372
context = mock<IExtensionContext>();
6473
shell = mock<IApplicationShell>();
65-
collection = mock<EnvironmentVariableCollection>();
74+
collection = mock<
75+
EnvironmentVariableCollection & {
76+
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
77+
}
78+
>();
79+
scopedCollection = mock<EnvironmentVariableCollection>();
80+
when(collection.getScopedEnvironmentVariableCollection(anything())).thenReturn(instance(scopedCollection));
6681
when(context.environmentVariableCollection).thenReturn(instance(collection));
6782
experimentService = mock<IExperimentService>();
6883
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
@@ -166,12 +181,12 @@ suite('Terminal Environment Variable Collection Service', () => {
166181
).thenResolve(envVars);
167182

168183
when(collection.replace(anything(), anything(), anything())).thenResolve();
169-
when(collection.delete(anything(), anything())).thenResolve();
184+
when(collection.delete(anything())).thenResolve();
170185

171186
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);
172187

173188
verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
174-
verify(collection.delete('PATH', anything())).once();
189+
verify(collection.delete('PATH')).once();
175190
});
176191

177192
test('Verify envs are not applied if env activation is disabled', async () => {
@@ -187,7 +202,7 @@ suite('Terminal Environment Variable Collection Service', () => {
187202
).thenResolve(envVars);
188203

189204
when(collection.replace(anything(), anything(), anything())).thenResolve();
190-
when(collection.delete(anything(), anything())).thenResolve();
205+
when(collection.delete(anything())).thenResolve();
191206
reset(configService);
192207
when(configService.getSettings(anything())).thenReturn(({
193208
terminal: { activateEnvironment: false },
@@ -197,10 +212,10 @@ suite('Terminal Environment Variable Collection Service', () => {
197212
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);
198213

199214
verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).never();
200-
verify(collection.delete('PATH', anything())).never();
215+
verify(collection.delete('PATH')).never();
201216
});
202217

203-
test('Verify correct scope is used when applying envs and setting description', async () => {
218+
test('Verify correct options are used when applying envs and setting description', async () => {
204219
const envVars: NodeJS.ProcessEnv = { CONDA_PREFIX: 'prefix/to/conda', ..._normCaseKeys(process.env) };
205220
delete envVars.PATH;
206221
const resource = Uri.file('a');
@@ -214,25 +229,16 @@ suite('Terminal Environment Variable Collection Service', () => {
214229
environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, customShell),
215230
).thenResolve(envVars);
216231

217-
when(collection.replace(anything(), anything(), anything())).thenCall((_e, _v, scope) => {
218-
assert.deepEqual(scope, { workspaceFolder });
219-
return Promise.resolve();
220-
});
221-
when(collection.delete(anything(), anything())).thenCall((_e, scope) => {
222-
assert.deepEqual(scope, { workspaceFolder });
232+
when(scopedCollection.replace(anything(), anything(), anything())).thenCall((_e, _v, options) => {
233+
assert.deepEqual(options, { applyAtShellIntegration: true });
223234
return Promise.resolve();
224235
});
225-
let description = '';
226-
when(collection.setDescription(anything(), anything())).thenCall((d, scope) => {
227-
assert.deepEqual(scope, { workspaceFolder });
228-
description = d.value;
229-
});
236+
when(scopedCollection.delete(anything())).thenResolve();
230237

231238
await terminalEnvVarCollectionService._applyCollection(resource, customShell);
232239

233-
verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
234-
verify(collection.delete('PATH', anything())).once();
235-
expect(description).to.equal(`${Interpreters.activateTerminalDescription} \`${displayPath}\``);
240+
verify(scopedCollection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
241+
verify(scopedCollection.delete('PATH')).once();
236242
});
237243

238244
test('Only relative changes to previously applied variables are applied to the collection', async () => {
@@ -251,7 +257,7 @@ suite('Terminal Environment Variable Collection Service', () => {
251257
).thenResolve(envVars);
252258

253259
when(collection.replace(anything(), anything(), anything())).thenResolve();
254-
when(collection.delete(anything(), anything())).thenResolve();
260+
when(collection.delete(anything())).thenResolve();
255261

256262
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);
257263

@@ -270,8 +276,8 @@ suite('Terminal Environment Variable Collection Service', () => {
270276

271277
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);
272278

273-
verify(collection.delete('CONDA_PREFIX', anything())).once();
274-
verify(collection.delete('RANDOM_VAR', anything())).once();
279+
verify(collection.delete('CONDA_PREFIX')).once();
280+
verify(collection.delete('RANDOM_VAR')).once();
275281
});
276282

277283
test('If no activated variables are returned for custom shell, fallback to using default shell', async () => {
@@ -294,12 +300,12 @@ suite('Terminal Environment Variable Collection Service', () => {
294300
).thenResolve(envVars);
295301

296302
when(collection.replace(anything(), anything(), anything())).thenResolve();
297-
when(collection.delete(anything(), anything())).thenResolve();
303+
when(collection.delete(anything())).thenResolve();
298304

299305
await terminalEnvVarCollectionService._applyCollection(undefined, customShell);
300306

301307
verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
302-
verify(collection.delete(anything(), anything())).never();
308+
verify(collection.delete(anything())).never();
303309
});
304310

305311
test('If no activated variables are returned for default shell, clear collection', async () => {
@@ -313,12 +319,10 @@ suite('Terminal Environment Variable Collection Service', () => {
313319
).thenResolve(undefined);
314320

315321
when(collection.replace(anything(), anything(), anything())).thenResolve();
316-
when(collection.delete(anything(), anything())).thenResolve();
317-
when(collection.setDescription(anything(), anything())).thenReturn();
322+
when(collection.delete(anything())).thenResolve();
318323

319324
await terminalEnvVarCollectionService._applyCollection(undefined, defaultShell?.shell);
320325

321-
verify(collection.clear(anything())).once();
322-
verify(collection.setDescription(anything(), anything())).never();
326+
verify(collection.clear()).once();
323327
});
324328
});

‎types/src/vscode-dts/vscode.proposed.envCollectionOptions.d.ts

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
declare module 'vscode' {
7+
8+
// https://github.com/microsoft/vscode/issues/179476
9+
10+
/**
11+
* Options applied to the mutator.
12+
*/
13+
export interface EnvironmentVariableMutatorOptions {
14+
/**
15+
* Apply to the environment just before the process is created.
16+
*
17+
* Defaults to true.
18+
*/
19+
applyAtProcessCreation?: boolean;
20+
21+
/**
22+
* Apply to the environment in the shell integration script. Note that this _will not_ apply
23+
* the mutator if shell integration is disabled or not working for some reason.
24+
*
25+
* Defaults to false.
26+
*/
27+
applyAtShellIntegration?: boolean;
28+
}
29+
30+
/**
31+
* A type of mutation and its value to be applied to an environment variable.
32+
*/
33+
export interface EnvironmentVariableMutator {
34+
/**
35+
* Options applied to the mutator.
36+
*/
37+
readonly options: EnvironmentVariableMutatorOptions;
38+
}
39+
40+
export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
41+
/**
42+
* @param options Options applied to the mutator.
43+
*/
44+
replace(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void;
45+
46+
/**
47+
* @param options Options applied to the mutator.
48+
*/
49+
append(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void;
50+
51+
/**
52+
* @param options Options applied to the mutator.
53+
*/
54+
prepend(variable: string, value: string, options?: EnvironmentVariableMutatorOptions): void;
55+
}
56+
}

‎types/vscode.proposed.envCollectionWorkspace.d.ts

+20-24
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,30 @@
55

66
declare module 'vscode' {
77

8-
// https://github.com/microsoft/vscode/issues/171173
8+
// https://github.com/microsoft/vscode/issues/182069
99

10-
export interface EnvironmentVariableMutator {
11-
readonly type: EnvironmentVariableMutatorType;
12-
readonly value: string;
13-
readonly scope: EnvironmentVariableScope | undefined;
14-
}
15-
16-
export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
17-
/**
18-
* Sets a description for the environment variable collection, this will be used to describe the changes in the UI.
19-
* @param description A description for the environment variable collection.
20-
* @param scope Specific scope to which this description applies to.
21-
*/
22-
setDescription(description: string | MarkdownString | undefined, scope?: EnvironmentVariableScope): void;
23-
replace(variable: string, value: string, scope?: EnvironmentVariableScope): void;
24-
append(variable: string, value: string, scope?: EnvironmentVariableScope): void;
25-
prepend(variable: string, value: string, scope?: EnvironmentVariableScope): void;
26-
get(variable: string, scope?: EnvironmentVariableScope): EnvironmentVariableMutator | undefined;
27-
delete(variable: string, scope?: EnvironmentVariableScope): void;
28-
clear(scope?: EnvironmentVariableScope): void;
29-
30-
}
10+
// export interface ExtensionContext {
11+
// /**
12+
// * Gets the extension's environment variable collection for this workspace, enabling changes
13+
// * to be applied to terminal environment variables.
14+
// *
15+
// * @param scope The scope to which the environment variable collection applies to.
16+
// */
17+
// readonly environmentVariableCollection: EnvironmentVariableCollection & { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection };
18+
// }
3119

3220
export type EnvironmentVariableScope = {
3321
/**
34-
* The workspace folder to which this collection applies to. If unspecified, collection applies to all workspace folders.
35-
*/
22+
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
23+
*/
3624
workspaceFolder?: WorkspaceFolder;
3725
};
26+
27+
export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
28+
/**
29+
* A description for the environment variable collection, this will be used to describe the
30+
* changes in the UI.
31+
*/
32+
description: string | MarkdownString | undefined;
33+
}
3834
}

0 commit comments

Comments
 (0)