Update proposed API for env collection (microsoft#21819)

For microsoft#20822

Blocked on
microsoft/vscode#171173 (comment)

package.json

@@ -46,7 +46,7 @@
  "theme": "dark"
  },
  "engines": {
- "vscode": "^1.82.0-20230809"
+ "vscode": "^1.82.0-20230823"
  },
  "enableTelemetry": false,
  "keywords": [

src/client/interpreter/activation/terminalEnvVarCollectionService.ts

@@ -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';
@@ -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';
@@ -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) {
@@ -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);
  }
@@ -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',
@@ -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 (

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

@@ -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 {
@@ -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,
@@ -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>();
@@ -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 };

types/vscode.proposed.envCollectionWorkspace.d.ts

@@ -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;
+ };
 }