Skip to content

Commit

Permalink
Update proposed API for env collection (microsoft#21819)
Browse files Browse the repository at this point in the history 
For microsoft#20822

Blocked on
microsoft/vscode#171173 (comment)
  • Loading branch information
@anthonykim1
Kartik Raj authored and anthonykim1 committed Sep 12, 2023
1 parent 247a047 commit ed93ade
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 70 deletions.
2 changes: 1 addition & 1 deletion package-lock.json
View file

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

2 changes: 1 addition & 1 deletion 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.82.0-20230809"
"vscode": "^1.82.0-20230823"
},
"enableTelemetry": false,
"keywords": [
Expand Down
93 changes: 58 additions & 35 deletions src/client/interpreter/activation/terminalEnvVarCollectionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@

import * as path from 'path';
import { inject, injectable } from 'inversify';
import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
import {
ProgressOptions,
ProgressLocation,
MarkdownString,
WorkspaceFolder,
GlobalEnvironmentVariableCollection,
EnvironmentVariableScope,
} from 'vscode';
import { pathExists } from 'fs-extra';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
Expand All @@ -20,7 +27,7 @@ import {
} from '../../common/types';
import { Deferred, createDeferred } from '../../common/utils/async';
import { Interpreters } from '../../common/utils/localize';
import { traceDecoratorVerbose, traceVerbose, traceWarn } from '../../logging';
import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
import { IInterpreterService } from '../contracts';
import { defaultShells } from './service';
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
Expand Down Expand Up @@ -61,52 +68,56 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
) {}

public async activate(resource: Resource): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
this.context.environmentVariableCollection.clear();
await this.handleMicroVenv(resource);
try {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
this.context.environmentVariableCollection.clear();
await this.handleMicroVenv(resource);
if (!this.registeredOnce) {
this.interpreterService.onDidChangeInterpreter(
async (r) => {
await this.handleMicroVenv(r);
},
this,
this.disposables,
);
this.registeredOnce = true;
}
return;
}
if (!this.registeredOnce) {
this.interpreterService.onDidChangeInterpreter(
async (r) => {
await this.handleMicroVenv(r);
this.showProgress();
await this._applyCollection(r).ignoreErrors();
this.hideProgress();
},
this,
this.disposables,
);
this.applicationEnvironment.onDidChangeShell(
async (shell: string) => {
this.showProgress();
this.processEnvVars = undefined;
// Pass in the shell where known instead of relying on the application environment, because of bug
// on VSCode: https://github.com/microsoft/vscode/issues/160694
await this._applyCollection(undefined, shell).ignoreErrors();
this.hideProgress();
},
this,
this.disposables,
);
this.registeredOnce = true;
}
return;
}
if (!this.registeredOnce) {
this.interpreterService.onDidChangeInterpreter(
async (r) => {
this.showProgress();
await this._applyCollection(r).ignoreErrors();
this.hideProgress();
},
this,
this.disposables,
);
this.applicationEnvironment.onDidChangeShell(
async (shell: string) => {
this.showProgress();
this.processEnvVars = undefined;
// Pass in the shell where known instead of relying on the application environment, because of bug
// on VSCode: https://github.com/microsoft/vscode/issues/160694
await this._applyCollection(undefined, shell).ignoreErrors();
this.hideProgress();
},
this,
this.disposables,
);
this.registeredOnce = true;
this._applyCollection(resource).ignoreErrors();
} catch (ex) {
traceError(`Activating terminal env collection failed`, ex);
}
this._applyCollection(resource).ignoreErrors();
}

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.context.getEnvironmentVariableCollection({ workspaceFolder });
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
// Clear any previously set env vars from collection
envVarCollection.clear();
if (!settings.terminal.activateEnvironment) {
Expand Down Expand Up @@ -221,6 +232,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
// PS1 should be set but no PS1 was set.
return;
}
const config = this.workspaceService
.getConfiguration('terminal')
.get<boolean>('integrated.shellIntegration.enabled');
if (!config) {
traceVerbose('PS1 is not set when shell integration is disabled.');
return;
}
}
this.terminalPromptIsCorrect(resource);
}
Expand All @@ -232,7 +250,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
if (interpreter?.envType === EnvironmentType.Venv) {
const activatePath = path.join(path.dirname(interpreter.path), 'activate');
if (!(await pathExists(activatePath))) {
const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
const pathVarName = getSearchPathEnvVarNames()[0];
envVarCollection.replace(
'PATH',
Expand All @@ -241,13 +259,18 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
);
return;
}
this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
this.getEnvironmentVariableCollection({ workspaceFolder }).clear();
}
} catch (ex) {
traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex);
}
}

private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) {
const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection;
return envVarCollection.getScoped(scope);
}

private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
if (
Expand Down
50 changes: 40 additions & 10 deletions src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { mock, instance, when, anything, verify, reset } from 'ts-mockito';
import {
EnvironmentVariableCollection,
EnvironmentVariableMutatorOptions,
EnvironmentVariableScope,
GlobalEnvironmentVariableCollection,
ProgressLocation,
Uri,
WorkspaceConfiguration,
WorkspaceFolder,
} from 'vscode';
import {
Expand Down Expand Up @@ -44,12 +45,12 @@ suite('Terminal Environment Variable Collection Service', () => {
let context: IExtensionContext;
let shell: IApplicationShell;
let experimentService: IExperimentService;
let collection: EnvironmentVariableCollection & {
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
};
let collection: EnvironmentVariableCollection;
let globalCollection: GlobalEnvironmentVariableCollection;
let applicationEnvironment: IApplicationEnvironment;
let environmentActivationService: IEnvironmentActivationService;
let workspaceService: IWorkspaceService;
let workspaceConfig: WorkspaceConfiguration;
let terminalEnvVarCollectionService: TerminalEnvVarCollectionService;
const progressOptions = {
location: ProgressLocation.Window,
Expand All @@ -62,19 +63,20 @@ suite('Terminal Environment Variable Collection Service', () => {

setup(() => {
workspaceService = mock<IWorkspaceService>();
workspaceConfig = mock<WorkspaceConfiguration>();
when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined);
when(workspaceService.workspaceFolders).thenReturn(undefined);
when(workspaceService.getConfiguration('terminal')).thenReturn(instance(workspaceConfig));
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(true);
platform = mock<IPlatformService>();
when(platform.osType).thenReturn(getOSType());
interpreterService = mock<IInterpreterService>();
context = mock<IExtensionContext>();
shell = mock<IApplicationShell>();
collection = mock<
EnvironmentVariableCollection & {
getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
}
>();
when(context.getEnvironmentVariableCollection(anything())).thenReturn(instance(collection));
globalCollection = mock<GlobalEnvironmentVariableCollection>();
collection = mock<EnvironmentVariableCollection>();
when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
when(globalCollection.getScoped(anything())).thenReturn(instance(collection));
experimentService = mock<IExperimentService>();
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
applicationEnvironment = mock<IApplicationEnvironment>();
Expand Down Expand Up @@ -291,6 +293,34 @@ suite('Terminal Environment Variable Collection Service', () => {
expect(result).to.equal(true);
});

test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
reset(workspaceConfig);
when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(false);
when(platform.osType).thenReturn(OSType.Linux);
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
const ps1Shell = 'bash';
const resource = Uri.file('a');
const workspaceFolder: WorkspaceFolder = {
uri: Uri.file('workspacePath'),
name: 'workspace1',
index: 0,
};
when(interpreterService.getActiveInterpreter(resource)).thenResolve(({
type: PythonEnvType.Virtual,
} as unknown) as PythonEnvironment);
when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder);
when(
environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, ps1Shell),
).thenResolve(envVars);
when(collection.replace(anything(), anything(), anything())).thenReturn();

await terminalEnvVarCollectionService._applyCollection(resource, ps1Shell);

const result = terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource);

expect(result).to.equal(false);
});

test('Correct track that prompt was not set for non-Windows zsh where PS1 is set', async () => {
when(platform.osType).thenReturn(OSType.Linux);
const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
Expand Down
50 changes: 27 additions & 23 deletions types/vscode.proposed.envCollectionWorkspace.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,34 @@
*--------------------------------------------------------------------------------------------*/

declare module 'vscode' {
// https://github.com/microsoft/vscode/issues/171173

// https://github.com/microsoft/vscode/issues/182069
// export interface ExtensionContext {
// /**
// * Gets the extension's global environment variable collection for this workspace, enabling changes to be
// * applied to terminal environment variables.
// */
// readonly environmentVariableCollection: GlobalEnvironmentVariableCollection;
// }

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 interface GlobalEnvironmentVariableCollection extends EnvironmentVariableCollection {
/**
* Gets scope-specific environment variable collection for the extension. This enables alterations to
* terminal environment variables solely within the designated scope, and is applied in addition to (and
* after) the global collection.
*
* Each object obtained through this method is isolated and does not impact objects for other scopes,
* including the global collection.
*
* @param scope The scope to which the environment variable collection applies to.
*/
getScoped(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 type EnvironmentVariableScope = {
/**
* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
*/
workspaceFolder?: WorkspaceFolder;
};
}

0 comments on commit ed93ade

Please sign in to comment.