Skip to content

Commit 30e26c2

Browse files
author
Kartik Raj
authored
Update proposed API for env collection (#21819)
For #20822 Blocked on microsoft/vscode#171173 (comment)
1 parent 15bb974 commit 30e26c2

File tree

5 files changed

+127
-70
lines changed

5 files changed

+127
-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

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
"theme": "dark"
4747
},
4848
"engines": {
49-
"vscode": "^1.82.0-20230809"
49+
"vscode": "^1.82.0-20230823"
5050
},
5151
"enableTelemetry": false,
5252
"keywords": [

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

+58-35
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+
GlobalEnvironmentVariableCollection,
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';
@@ -20,7 +27,7 @@ import {
2027
} from '../../common/types';
2128
import { Deferred, createDeferred } from '../../common/utils/async';
2229
import { Interpreters } from '../../common/utils/localize';
23-
import { traceDecoratorVerbose, traceVerbose, traceWarn } from '../../logging';
30+
import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
2431
import { IInterpreterService } from '../contracts';
2532
import { defaultShells } from './service';
2633
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
@@ -61,52 +68,56 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
6168
) {}
6269

6370
public async activate(resource: Resource): Promise<void> {
64-
if (!inTerminalEnvVarExperiment(this.experimentService)) {
65-
this.context.environmentVariableCollection.clear();
66-
await this.handleMicroVenv(resource);
71+
try {
72+
if (!inTerminalEnvVarExperiment(this.experimentService)) {
73+
this.context.environmentVariableCollection.clear();
74+
await this.handleMicroVenv(resource);
75+
if (!this.registeredOnce) {
76+
this.interpreterService.onDidChangeInterpreter(
77+
async (r) => {
78+
await this.handleMicroVenv(r);
79+
},
80+
this,
81+
this.disposables,
82+
);
83+
this.registeredOnce = true;
84+
}
85+
return;
86+
}
6787
if (!this.registeredOnce) {
6888
this.interpreterService.onDidChangeInterpreter(
6989
async (r) => {
70-
await this.handleMicroVenv(r);
90+
this.showProgress();
91+
await this._applyCollection(r).ignoreErrors();
92+
this.hideProgress();
93+
},
94+
this,
95+
this.disposables,
96+
);
97+
this.applicationEnvironment.onDidChangeShell(
98+
async (shell: string) => {
99+
this.showProgress();
100+
this.processEnvVars = undefined;
101+
// Pass in the shell where known instead of relying on the application environment, because of bug
102+
// on VSCode: https://github.com/microsoft/vscode/issues/160694
103+
await this._applyCollection(undefined, shell).ignoreErrors();
104+
this.hideProgress();
71105
},
72106
this,
73107
this.disposables,
74108
);
75109
this.registeredOnce = true;
76110
}
77-
return;
78-
}
79-
if (!this.registeredOnce) {
80-
this.interpreterService.onDidChangeInterpreter(
81-
async (r) => {
82-
this.showProgress();
83-
await this._applyCollection(r).ignoreErrors();
84-
this.hideProgress();
85-
},
86-
this,
87-
this.disposables,
88-
);
89-
this.applicationEnvironment.onDidChangeShell(
90-
async (shell: string) => {
91-
this.showProgress();
92-
this.processEnvVars = undefined;
93-
// Pass in the shell where known instead of relying on the application environment, because of bug
94-
// on VSCode: https://github.com/microsoft/vscode/issues/160694
95-
await this._applyCollection(undefined, shell).ignoreErrors();
96-
this.hideProgress();
97-
},
98-
this,
99-
this.disposables,
100-
);
101-
this.registeredOnce = true;
111+
this._applyCollection(resource).ignoreErrors();
112+
} catch (ex) {
113+
traceError(`Activating terminal env collection failed`, ex);
102114
}
103-
this._applyCollection(resource).ignoreErrors();
104115
}
105116

106117
public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
107118
const workspaceFolder = this.getWorkspaceFolder(resource);
108119
const settings = this.configurationService.getSettings(resource);
109-
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
120+
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
110121
// Clear any previously set env vars from collection
111122
envVarCollection.clear();
112123
if (!settings.terminal.activateEnvironment) {
@@ -221,6 +232,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
221232
// PS1 should be set but no PS1 was set.
222233
return;
223234
}
235+
const config = this.workspaceService
236+
.getConfiguration('terminal')
237+
.get<boolean>('integrated.shellIntegration.enabled');
238+
if (!config) {
239+
traceVerbose('PS1 is not set when shell integration is disabled.');
240+
return;
241+
}
224242
}
225243
this.terminalPromptIsCorrect(resource);
226244
}
@@ -232,7 +250,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
232250
if (interpreter?.envType === EnvironmentType.Venv) {
233251
const activatePath = path.join(path.dirname(interpreter.path), 'activate');
234252
if (!(await pathExists(activatePath))) {
235-
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
253+
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
236254
const pathVarName = getSearchPathEnvVarNames()[0];
237255
envVarCollection.replace(
238256
'PATH',
@@ -241,13 +259,18 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
241259
);
242260
return;
243261
}
244-
this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
262+
this.getEnvironmentVariableCollection({ workspaceFolder }).clear();
245263
}
246264
} catch (ex) {
247265
traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex);
248266
}
249267
}
250268

269+
private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) {
270+
const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection;
271+
return envVarCollection.getScoped(scope);
272+
}
273+
251274
private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
252275
let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
253276
if (

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

+40-10
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ import { mock, instance, when, anything, verify, reset } from 'ts-mockito';
99
import {
1010
EnvironmentVariableCollection,
1111
EnvironmentVariableMutatorOptions,
12-
EnvironmentVariableScope,
12+
GlobalEnvironmentVariableCollection,
1313
ProgressLocation,
1414
Uri,
15+
WorkspaceConfiguration,
1516
WorkspaceFolder,
1617
} from 'vscode';
1718
import {
@@ -44,12 +45,12 @@ suite('Terminal Environment Variable Collection Service', () => {
4445
let context: IExtensionContext;
4546
let shell: IApplicationShell;
4647
let experimentService: IExperimentService;
47-
let collection: EnvironmentVariableCollection & {
48-
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
49-
};
48+
let collection: EnvironmentVariableCollection;
49+
let globalCollection: GlobalEnvironmentVariableCollection;
5050
let applicationEnvironment: IApplicationEnvironment;
5151
let environmentActivationService: IEnvironmentActivationService;
5252
let workspaceService: IWorkspaceService;
53+
let workspaceConfig: WorkspaceConfiguration;
5354
let terminalEnvVarCollectionService: TerminalEnvVarCollectionService;
5455
const progressOptions = {
5556
location: ProgressLocation.Window,
@@ -62,19 +63,20 @@ suite('Terminal Environment Variable Collection Service', () => {
6263

6364
setup(() => {
6465
workspaceService = mock<IWorkspaceService>();
66+
workspaceConfig = mock<WorkspaceConfiguration>();
6567
when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined);
6668
when(workspaceService.workspaceFolders).thenReturn(undefined);
69+
when(workspaceService.getConfiguration('terminal')).thenReturn(instance(workspaceConfig));
70+
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(true);
6771
platform = mock<IPlatformService>();
6872
when(platform.osType).thenReturn(getOSType());
6973
interpreterService = mock<IInterpreterService>();
7074
context = mock<IExtensionContext>();
7175
shell = mock<IApplicationShell>();
72-
collection = mock<
73-
EnvironmentVariableCollection & {
74-
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
75-
}
76-
>();
77-
when(context.getEnvironmentVariableCollection(anything())).thenReturn(instance(collection));
76+
globalCollection = mock<GlobalEnvironmentVariableCollection>();
77+
collection = mock<EnvironmentVariableCollection>();
78+
when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
79+
when(globalCollection.getScoped(anything())).thenReturn(instance(collection));
7880
experimentService = mock<IExperimentService>();
7981
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
8082
applicationEnvironment = mock<IApplicationEnvironment>();
@@ -291,6 +293,34 @@ suite('Terminal Environment Variable Collection Service', () => {
291293
expect(result).to.equal(true);
292294
});
293295

296+
test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
297+
reset(workspaceConfig);
298+
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(false);
299+
when(platform.osType).thenReturn(OSType.Linux);
300+
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
301+
const ps1Shell = 'bash';
302+
const resource = Uri.file('a');
303+
const workspaceFolder: WorkspaceFolder = {
304+
uri: Uri.file('workspacePath'),
305+
name: 'workspace1',
306+
index: 0,
307+
};
308+
when(interpreterService.getActiveInterpreter(resource)).thenResolve(({
309+
type: PythonEnvType.Virtual,
310+
} as unknown) as PythonEnvironment);
311+
when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder);
312+
when(
313+
environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, ps1Shell),
314+
).thenResolve(envVars);
315+
when(collection.replace(anything(), anything(), anything())).thenReturn();
316+
317+
await terminalEnvVarCollectionService._applyCollection(resource, ps1Shell);
318+
319+
const result = terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource);
320+
321+
expect(result).to.equal(false);
322+
});
323+
294324
test('Correct track that prompt was not set for non-Windows zsh where PS1 is set', async () => {
295325
when(platform.osType).thenReturn(OSType.Linux);
296326
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };

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

+27-23
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,34 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
declare module 'vscode' {
7+
// https://github.com/microsoft/vscode/issues/171173
78

8-
// https://github.com/microsoft/vscode/issues/182069
9+
// export interface ExtensionContext {
10+
// /**
11+
// * Gets the extension's global environment variable collection for this workspace, enabling changes to be
12+
// * applied to terminal environment variables.
13+
// */
14+
// readonly environmentVariableCollection: GlobalEnvironmentVariableCollection;
15+
// }
916

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-
* @deprecated Use {@link getEnvironmentVariableCollection} instead.
16-
*/
17-
readonly environmentVariableCollection: EnvironmentVariableCollection;
18-
/**
19-
* Gets the extension's environment variable collection for this workspace, enabling changes
20-
* to be applied to terminal environment variables.
21-
*
22-
* @param scope The scope to which the environment variable collection applies to.
23-
*/
24-
getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;
25-
}
17+
export interface GlobalEnvironmentVariableCollection extends EnvironmentVariableCollection {
18+
/**
19+
* Gets scope-specific environment variable collection for the extension. This enables alterations to
20+
* terminal environment variables solely within the designated scope, and is applied in addition to (and
21+
* after) the global collection.
22+
*
23+
* Each object obtained through this method is isolated and does not impact objects for other scopes,
24+
* including the global collection.
25+
*
26+
* @param scope The scope to which the environment variable collection applies to.
27+
*/
28+
getScoped(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
29+
}
2630

27-
export type EnvironmentVariableScope = {
28-
/**
29-
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
30-
*/
31-
workspaceFolder?: WorkspaceFolder;
32-
};
31+
export type EnvironmentVariableScope = {
32+
/**
33+
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
34+
*/
35+
workspaceFolder?: WorkspaceFolder;
36+
};
3337
}

0 commit comments

Comments
 (0)