Skip to content

Commit

Permalink
Support pythonTerminalEnvVarActivation experiment in multiroot work…
Browse files Browse the repository at this point in the history
…spaces (#21047)
  • Loading branch information
Kartik Raj committed Apr 23, 2023
1 parent d905cf0 commit 9f24fbf
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 65 deletions.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"quickPickSortByLabel",
"envShellEvent",
"testObserver",
"quickPickItemTooltip"
"quickPickItemTooltip",
"envCollectionWorkspace"
],
"author": {
"name": "Microsoft Corporation"
Expand All @@ -43,7 +44,7 @@
"theme": "dark"
},
"engines": {
"vscode": "^1.77.0-20230309"
"vscode": "^1.78.0-20230421"
},
"keywords": [
"python",
Expand Down
7 changes: 0 additions & 7 deletions src/client/common/experiments/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,10 @@

'use strict';

import { workspace } from 'vscode';
import { isTestExecution } from '../constants';
import { IExperimentService } from '../types';
import { TerminalEnvVarActivation } from './groups';

export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean {
if (workspace.workspaceFile && !isTestExecution()) {
// Don't run experiment in multi-root workspaces for now, requires work on VSCode:
// https://github.com/microsoft/vscode/issues/171173
return false;
}
if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export namespace Interpreters {
'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change "terminal.integrated.inheritEnv" to false in your user settings. [Learn more](https://aka.ms/AA66i8f).',
);
export const activatingTerminals = l10n.t('Reactivating terminals...');
export const activateTerminalDescription = l10n.t('Activated environment for');
export const activatedCondaEnvLaunch = l10n.t(
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { ProgressOptions, ProgressLocation } from 'vscode';
import { IExtensionSingleActivationService } from '../../activation/types';
import { IApplicationShell, IApplicationEnvironment } from '../../common/application/types';
import { ProgressOptions, ProgressLocation, MarkdownString } from 'vscode';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { IPlatformService } from '../../common/platform/types';
import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';
Expand All @@ -14,6 +14,7 @@ import {
Resource,
IDisposableRegistry,
IConfigurationService,
IPathUtils,
} from '../../common/types';
import { Deferred, createDeferred } from '../../common/utils/async';
import { Interpreters } from '../../common/utils/localize';
Expand All @@ -23,14 +24,16 @@ import { defaultShells } from './service';
import { IEnvironmentActivationService } from './types';

@injectable()
export class TerminalEnvVarCollectionService implements IExtensionSingleActivationService {
export class TerminalEnvVarCollectionService implements IExtensionActivationService {
public readonly supportedWorkspaceTypes = {
untrustedWorkspace: false,
virtualWorkspace: false,
};

private deferred: Deferred<void> | undefined;

private registeredOnce = false;

private previousEnvVars = _normCaseKeys(process.env);

constructor(
Expand All @@ -42,39 +45,51 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati
@inject(IApplicationEnvironment) private applicationEnvironment: IApplicationEnvironment,
@inject(IDisposableRegistry) private disposables: IDisposableRegistry,
@inject(IEnvironmentActivationService) private environmentActivationService: IEnvironmentActivationService,
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
@inject(IPathUtils) private readonly pathUtils: IPathUtils,
) {}

public async activate(): Promise<void> {
public async activate(resource: Resource): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
this.context.environmentVariableCollection.clear();
return;
}
this.interpreterService.onDidChangeInterpreter(
async (resource) => {
this.showProgress();
await this._applyCollection(resource);
this.hideProgress();
},
this,
this.disposables,
);
this.applicationEnvironment.onDidChangeShell(
async (shell: string) => {
this.showProgress();
// 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);
this.hideProgress();
},
this,
this.disposables,
);

this._applyCollection(undefined).ignoreErrors();
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();
// 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();
}

public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
if (
!workspaceFolder &&
Array.isArray(this.workspaceService.workspaceFolders) &&
this.workspaceService.workspaceFolders.length > 0
) {
[workspaceFolder] = this.workspaceService.workspaceFolders;
}
const settings = this.configurationService.getSettings(resource);
if (!settings.terminal.activateEnvironment) {
traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath);
Expand All @@ -95,7 +110,7 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati
await this._applyCollection(resource, defaultShell?.shell);
return;
}
this.context.environmentVariableCollection.clear();
this.context.environmentVariableCollection.clear({ workspaceFolder });
this.previousEnvVars = _normCaseKeys(process.env);
return;
}
Expand All @@ -107,20 +122,25 @@ export class TerminalEnvVarCollectionService implements IExtensionSingleActivati
if (prevValue !== value) {
if (value !== undefined) {
traceVerbose(`Setting environment variable ${key} in collection to ${value}`);
this.context.environmentVariableCollection.replace(key, value);
this.context.environmentVariableCollection.replace(key, value, { workspaceFolder });
} else {
traceVerbose(`Clearing environment variable ${key} from collection`);
this.context.environmentVariableCollection.delete(key);
this.context.environmentVariableCollection.delete(key, { workspaceFolder });
}
}
});
Object.keys(previousEnv).forEach((key) => {
// If the previous env var is not in the current env, clear it from collection.
if (!(key in env)) {
traceVerbose(`Clearing environment variable ${key} from collection`);
this.context.environmentVariableCollection.delete(key);
this.context.environmentVariableCollection.delete(key, { workspaceFolder });
}
});
const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath);
const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``);
this.context.environmentVariableCollection.setDescription(description, {
workspaceFolder,
});
}

@traceDecoratorVerbose('Display activating terminals')
Expand Down
4 changes: 2 additions & 2 deletions src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ export function registerTypes(serviceManager: IServiceManager): void {
IEnvironmentActivationService,
EnvironmentActivationService,
);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
serviceManager.addSingleton<IExtensionActivationService>(
IExtensionActivationService,
TerminalEnvVarCollectionService,
);
}
Loading

0 comments on commit 9f24fbf

Please sign in to comment.