Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pythonTerminalEnvVarActivation experiment in multiroot workspaces #21047

Merged
merged 21 commits into from
Apr 23, 2023
Merged
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