Skip to content

Commit

Permalink
Use updated API to fetch scoped env collection (#21788)
Browse files Browse the repository at this point in the history 
For microsoft/vscode#171173
#20822

To be merged tomorrow when latest insiders is released. Blocked on
microsoft/vscode#189979.
  • Loading branch information
Kartik Raj authored Aug 10, 2023
1 parent 0a2c285 commit fbbf987
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 61 deletions.
16 changes: 8 additions & 8 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"theme": "dark"
},
"engines": {
"vscode": "^1.79.0-20230526"
"vscode": "^1.81.0-20230809"
},
"enableTelemetry": false,
"keywords": [
Expand Down Expand Up @@ -2151,7 +2151,7 @@
"@types/stack-trace": "0.0.29",
"@types/tmp": "^0.0.33",
"@types/uuid": "^8.3.4",
"@types/vscode": "^1.75.0",
"@types/vscode": "^1.81.0",
"@types/which": "^2.0.1",
"@types/winreg": "^1.2.30",
"@types/xml2js": "^0.4.2",
Expand Down
34 changes: 11 additions & 23 deletions src/client/interpreter/activation/terminalEnvVarCollectionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,7 @@

import * as path from 'path';
import { inject, injectable } from 'inversify';
import {
ProgressOptions,
ProgressLocation,
MarkdownString,
WorkspaceFolder,
EnvironmentVariableCollection,
EnvironmentVariableScope,
} from 'vscode';
import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
import { pathExists } from 'fs-extra';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
Expand Down Expand Up @@ -67,7 +60,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ

public async activate(resource: Resource): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
this.context.environmentVariableCollection.clear();
const workspaceFolder = this.getWorkspaceFolder(resource);
this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
await this.handleMicroVenv(resource);
if (!this.registeredOnce) {
this.interpreterService.onDidChangeInterpreter(
Expand Down Expand Up @@ -111,8 +105,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
const workspaceFolder = this.getWorkspaceFolder(resource);
const settings = this.configurationService.getSettings(resource);
const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder);
// Clear any previously set env vars from collection.
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
// Clear any previously set env vars from collection
envVarCollection.clear();
if (!settings.terminal.activateEnvironment) {
traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath);
Expand Down Expand Up @@ -160,7 +154,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
return;
}
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
envVarCollection.replace(key, value, { applyAtShellIntegration: true });
envVarCollection.replace(key, value, {
applyAtShellIntegration: true,
applyAtProcessCreation: true,
});
}
}
});
Expand All @@ -170,22 +167,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
envVarCollection.description = description;
}

private getEnvironmentVariableCollection(workspaceFolder?: WorkspaceFolder) {
const envVarCollection = this.context.environmentVariableCollection as EnvironmentVariableCollection & {
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
};
return workspaceFolder
? envVarCollection.getScopedEnvironmentVariableCollection({ workspaceFolder })
: envVarCollection;
}

private async handleMicroVenv(resource: Resource) {
const workspaceFolder = this.getWorkspaceFolder(resource);
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
if (interpreter?.envType === EnvironmentType.Venv) {
const activatePath = path.join(path.dirname(interpreter.path), 'activate');
if (!(await pathExists(activatePath))) {
const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder);
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
const pathVarName = getSearchPathEnvVarNames()[0];
envVarCollection.replace(
'PATH',
Expand All @@ -195,7 +183,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
return;
}
}
this.context.environmentVariableCollection.clear();
this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
}

private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
Expand Down
20 changes: 9 additions & 11 deletions src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ suite('Terminal Environment Variable Collection Service', () => {
let collection: EnvironmentVariableCollection & {
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
};
let scopedCollection: EnvironmentVariableCollection;
let applicationEnvironment: IApplicationEnvironment;
let environmentActivationService: IEnvironmentActivationService;
let workspaceService: IWorkspaceService;
Expand Down Expand Up @@ -73,9 +72,7 @@ suite('Terminal Environment Variable Collection Service', () => {
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
}
>();
scopedCollection = mock<EnvironmentVariableCollection>();
when(collection.getScopedEnvironmentVariableCollection(anything())).thenReturn(instance(scopedCollection));
when(context.environmentVariableCollection).thenReturn(instance(collection));
when(context.getEnvironmentVariableCollection(anything())).thenReturn(instance(collection));
experimentService = mock<IExperimentService>();
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
applicationEnvironment = mock<IApplicationEnvironment>();
Expand All @@ -95,7 +92,6 @@ suite('Terminal Environment Variable Collection Service', () => {
pythonPath: displayPath,
} as unknown) as IPythonSettings);
when(collection.clear()).thenResolve();
when(scopedCollection.clear()).thenResolve();
terminalEnvVarCollectionService = new TerminalEnvVarCollectionService(
instance(platform),
instance(interpreterService),
Expand Down Expand Up @@ -253,15 +249,17 @@ suite('Terminal Environment Variable Collection Service', () => {
environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, customShell),
).thenResolve(envVars);

when(scopedCollection.replace(anything(), anything(), anything())).thenCall((_e, _v, options) => {
assert.deepEqual(options, { applyAtShellIntegration: true });
return Promise.resolve();
});
when(collection.replace(anything(), anything(), anything())).thenCall(
(_e, _v, options: EnvironmentVariableMutatorOptions) => {
assert.deepEqual(options, { applyAtShellIntegration: true, applyAtProcessCreation: true });
return Promise.resolve();
},
);

await terminalEnvVarCollectionService._applyCollection(resource, customShell);

verify(scopedCollection.clear()).once();
verify(scopedCollection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
verify(collection.clear()).once();
verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once();
});

test('If no activated variables are returned for custom shell, fallback to using default shell', async () => {
Expand Down
33 changes: 16 additions & 17 deletions types/vscode.proposed.envCollectionWorkspace.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,27 @@ declare module 'vscode' {

// https://github.com/microsoft/vscode/issues/182069

// export interface ExtensionContext {
// /**
// * Gets the extension's environment variable collection for this workspace, enabling changes
// * to be applied to terminal environment variables.
// *
// * @param scope The scope to which the environment variable collection applies to.
// */
// readonly environmentVariableCollection: EnvironmentVariableCollection & { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection };
// }
export interface ExtensionContext {
/**
* Gets the extension's environment variable collection for this workspace, enabling changes
* to be applied to terminal environment variables.
*
* @deprecated Use {@link getEnvironmentVariableCollection} instead.
*/
readonly environmentVariableCollection: EnvironmentVariableCollection;
/**
* Gets the extension's environment variable collection for this workspace, enabling changes
* to be applied to terminal environment variables.
*
* @param scope The scope to which the environment variable collection applies to.
*/
getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection;
}

export type EnvironmentVariableScope = {
/**
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
*/
workspaceFolder?: WorkspaceFolder;
};

export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> {
/**
* A description for the environment variable collection, this will be used to describe the
* changes in the UI.
*/
description: string | MarkdownString | undefined;
}
}

0 comments on commit fbbf987

Please sign in to comment.