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

Show notification reaffirming Python extension still handles activation when in pythonTerminalEnvVarActivation experiment #21802

Merged
merged 18 commits into from
Aug 15, 2023
4 changes: 2 additions & 2 deletions src/client/activation/extensionSurvey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ export class ExtensionSurveyPrompt implements IExtensionSingleActivationService
@traceDecoratorError('Failed to display prompt for extension survey')
public async showSurvey() {
const prompts = [ExtensionSurveyBanner.bannerLabelYes, ExtensionSurveyBanner.maybeLater, Common.doNotShowAgain];
const telemetrySelections: ['Yes', 'Maybe later', 'Do not show again'] = [
const telemetrySelections: ['Yes', 'Maybe later', "Don't show again"] = [
'Yes',
'Maybe later',
'Do not show again',
"Don't show again",
];
const selection = await this.appShell.showInformationMessage(ExtensionSurveyBanner.bannerMessage, ...prompts);
sendTelemetryEvent(EventName.EXTENSION_SURVEY_PROMPT, undefined, {
Expand Down
6 changes: 6 additions & 0 deletions src/client/common/experiments/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,16 @@

'use strict';

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

export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean {
if (!isTestExecution() && workspace.workspaceFile && env.remoteName) {
// TODO: Remove this if statement once https://github.com/microsoft/vscode/issues/180486 is fixed.
return false;
}
if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) {
return false;
}
Expand Down
5 changes: 4 additions & 1 deletion src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export namespace Common {
export const openOutputPanel = l10n.t('Show output');
export const noIWillDoItLater = l10n.t('No, I will do it later');
export const notNow = l10n.t('Not now');
export const doNotShowAgain = l10n.t('Do not show again');
export const doNotShowAgain = l10n.t("Don't show again");
export const reload = l10n.t('Reload');
export const moreInfo = l10n.t('More Info');
export const learnMore = l10n.t('Learn more');
Expand Down Expand Up @@ -198,6 +198,9 @@ export namespace Interpreters {
);
export const activatingTerminals = l10n.t('Reactivating terminals...');
export const activateTerminalDescription = l10n.t('Activated environment for');
export const terminalEnvVarCollectionPrompt = l10n.t(
'The Python extension automatically activates all terminals using the selected environment. You can hover over the terminal tab to see more information about the activation. [Learn more](https://aka.ms/vscodePythonTerminalActivation).',
);
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
@@ -0,0 +1,80 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { IActiveResourceService, IApplicationShell, ITerminalManager } from '../../common/application/types';
import {
IConfigurationService,
IDisposableRegistry,
IExperimentService,
IPersistentStateFactory,
} from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { ITerminalEnvVarCollectionService } from './types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';

export const terminalEnvCollectionPromptKey = 'TERMINAL_ENV_COLLECTION_PROMPT_KEY';

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

constructor(
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
@inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry,
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
@inject(ITerminalEnvVarCollectionService)
private readonly terminalEnvVarCollectionService: ITerminalEnvVarCollectionService,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {}

public async activate(): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
return;
}
this.disposableRegistry.push(
this.terminalManager.onDidOpenTerminal(async (terminal) => {
const cwd =
'cwd' in terminal.creationOptions && terminal.creationOptions.cwd
? terminal.creationOptions.cwd
: this.activeResourceService.getActiveResource();
const resource = typeof cwd === 'string' ? Uri.file(cwd) : cwd;
const settings = this.configurationService.getSettings(resource);
if (!settings.terminal.activateEnvironment) {
return;
}
if (this.terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource)) {
// No need to show notification if terminal prompt already indicates when env is activated.
return;
}
await this.notifyUsers();
}),
);
}

private async notifyUsers(): Promise<void> {
const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(
terminalEnvCollectionPromptKey,
true,
);
if (!notificationPromptEnabled.value) {
return;
}
const prompts = [Common.doNotShowAgain];
const selection = await this.appShell.showInformationMessage(
Interpreters.terminalEnvVarCollectionPrompt,
...prompts,
);
if (!selection) {
return;
}
if (selection === prompts[0]) {
await notificationPromptEnabled.updateValue(false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import { Interpreters } from '../../common/utils/localize';
import { traceDecoratorVerbose, traceVerbose } from '../../logging';
import { IInterpreterService } from '../contracts';
import { defaultShells } from './service';
import { IEnvironmentActivationService } from './types';
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
import { EnvironmentType } from '../../pythonEnvironments/info';
import { getSearchPathEnvVarNames } from '../../common/utils/exec';
import { EnvironmentVariables } from '../../common/variables/types';
import { TerminalShellType } from '../../common/terminal/types';
import { OSType } from '../../common/utils/platform';

@injectable()
export class TerminalEnvVarCollectionService implements IExtensionActivationService {
export class TerminalEnvVarCollectionService implements IExtensionActivationService, ITerminalEnvVarCollectionService {
public readonly supportedWorkspaceTypes = {
untrustedWorkspace: false,
virtualWorkspace: false,
Expand Down Expand Up @@ -127,6 +129,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
await this._applyCollection(resource, defaultShell?.shell);
return;
}
await this.trackTerminalPrompt(shell, resource, env);
this.processEnvVars = undefined;
return;
}
Expand All @@ -146,6 +149,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
if (prevValue !== value) {
if (value !== undefined) {
if (key === 'PS1') {
// We cannot have the full PS1 without executing in terminal, which we do not. Hence prepend it.
traceVerbose(`Prepending environment variable ${key} in collection with ${value}`);
envVarCollection.prepend(key, value, {
applyAtShellIntegration: true,
Expand All @@ -165,6 +169,61 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
const displayPath = this.pathUtils.getDisplayName(settings.pythonPath, workspaceFolder?.uri.fsPath);
const description = new MarkdownString(`${Interpreters.activateTerminalDescription} \`${displayPath}\``);
envVarCollection.description = description;

await this.trackTerminalPrompt(shell, resource, env);
}

private isPromptSet = new Map<number | undefined, boolean>();

// eslint-disable-next-line class-methods-use-this
public isTerminalPromptSetCorrectly(resource?: Resource): boolean {
const workspaceFolder = this.getWorkspaceFolder(resource);
return !!this.isPromptSet.get(workspaceFolder?.index);
}

/**
* Call this once we know terminal prompt is set correctly for terminal owned by this resource.
*/
private terminalPromptIsCorrect(resource: Resource) {
const key = this.getWorkspaceFolder(resource)?.index;
this.isPromptSet.set(key, true);
}

private terminalPromptIsUnknown(resource: Resource) {
const key = this.getWorkspaceFolder(resource)?.index;
this.isPromptSet.delete(key);
}

/**
* Tracks whether prompt for terminal was correctly set.
*/
private async trackTerminalPrompt(shell: string, resource: Resource, env: EnvironmentVariables | undefined) {
this.terminalPromptIsUnknown(resource);
if (!env) {
this.terminalPromptIsCorrect(resource);
return;
}
// Prompts for these shells cannot be set reliably using variables
const exceptionShells = [
TerminalShellType.powershell,
TerminalShellType.powershellCore,
TerminalShellType.fish,
TerminalShellType.zsh, // TODO: Remove this once https://github.com/microsoft/vscode/issues/188875 is fixed
];
const customShellType = identifyShellFromShellPath(shell);
if (exceptionShells.includes(customShellType)) {
return;
}
if (this.platform.osType !== OSType.Windows) {
// These shells are expected to set PS1 variable for terminal prompt for virtual/conda environments.
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
const shouldPS1BeSet = interpreter?.type !== undefined;
if (shouldPS1BeSet && !env.PS1) {
// PS1 should be set but no PS1 was set.
return;
}
}
this.terminalPromptIsCorrect(resource);
}

private async handleMicroVenv(resource: Resource) {
Expand All @@ -178,7 +237,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
envVarCollection.replace(
'PATH',
`${path.dirname(interpreter.path)}${path.delimiter}${process.env[pathVarName]}`,
{ applyAtShellIntegration: true },
{ applyAtShellIntegration: true, applyAtProcessCreation: true },
);
return;
}
Expand Down
8 changes: 8 additions & 0 deletions src/client/interpreter/activation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ export interface IEnvironmentActivationService {
interpreter?: PythonEnvironment,
): Promise<string[] | undefined>;
}

export const ITerminalEnvVarCollectionService = Symbol('ITerminalEnvVarCollectionService');
export interface ITerminalEnvVarCollectionService {
/**
* Returns true if we know with high certainity the terminal prompt is set correctly for a particular resource.
*/
isTerminalPromptSetCorrectly(resource?: Resource): boolean;
}
12 changes: 9 additions & 3 deletions src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types';
import { IServiceManager } from '../ioc/types';
import { EnvironmentActivationService } from './activation/service';
import { TerminalEnvVarCollectionPrompt } from './activation/terminalEnvVarCollectionPrompt';
import { TerminalEnvVarCollectionService } from './activation/terminalEnvVarCollectionService';
import { IEnvironmentActivationService } from './activation/types';
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './activation/types';
import { InterpreterAutoSelectionService } from './autoSelection/index';
import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy';
import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './autoSelection/types';
Expand Down Expand Up @@ -109,8 +110,13 @@ export function registerTypes(serviceManager: IServiceManager): void {
IEnvironmentActivationService,
EnvironmentActivationService,
);
serviceManager.addSingleton<IExtensionActivationService>(
IExtensionActivationService,
serviceManager.addSingleton<ITerminalEnvVarCollectionService>(
ITerminalEnvVarCollectionService,
TerminalEnvVarCollectionService,
);
serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
TerminalEnvVarCollectionPrompt,
);
}
3 changes: 2 additions & 1 deletion src/client/pythonEnvironments/info/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
'use strict';

import { Architecture } from '../../common/utils/platform';
import { PythonEnvType } from '../base/info';
import { PythonVersion } from './pythonVersion';

/**
Expand Down Expand Up @@ -85,7 +86,7 @@ export type PythonEnvironment = InterpreterInformation & {
envName?: string;
envPath?: string;
cachedEntry?: boolean;
type?: string;
type?: PythonEnvType;
};

/**
Expand Down
8 changes: 4 additions & 4 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ export interface IEventNamePropertyMapping {
tool?: LinterId;
/**
* `select` When 'Select linter' option is selected
* `disablePrompt` When 'Do not show again' option is selected
* `disablePrompt` When "Don't show again" option is selected
* `install` When 'Install' option is selected
*
* @type {('select' | 'disablePrompt' | 'install')}
Expand Down Expand Up @@ -1369,7 +1369,7 @@ export interface IEventNamePropertyMapping {
/**
* `Yes` When 'Yes' option is selected
* `No` When 'No' option is selected
* `Ignore` When 'Do not show again' option is clicked
* `Ignore` When "Don't show again" option is clicked
*
* @type {('Yes' | 'No' | 'Ignore' | undefined)}
*/
Expand Down Expand Up @@ -1549,7 +1549,7 @@ export interface IEventNamePropertyMapping {
* This event also has a measure, "resultLength", which records the number of completions provided.
*/
/* __GDPR__
"jedi_language_server.request" : {
"jedi_language_server.request" : {
"method": {"classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "karthiknadig"}
}
*/
Expand All @@ -1566,7 +1566,7 @@ export interface IEventNamePropertyMapping {
/**
* Carries the selection of user when they are asked to take the extension survey
*/
selection: 'Yes' | 'Maybe later' | 'Do not show again' | undefined;
selection: 'Yes' | 'Maybe later' | "Don't show again" | undefined;
};
/**
* Telemetry event sent when starting REPL
Expand Down
2 changes: 1 addition & 1 deletion src/test/activation/extensionSurvey.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ suite('Extension survey prompt - showSurvey()', () => {
platformService.verifyAll();
});

test("Disable prompt if 'Do not show again' option is clicked", async () => {
test('Disable prompt if "Don\'t show again" option is clicked', async () => {
const prompts = [ExtensionSurveyBanner.bannerLabelYes, ExtensionSurveyBanner.maybeLater, Common.doNotShowAgain];
platformService.setup((p) => p.osType).verifiable(TypeMoq.Times.never());
appShell
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ suite('Application Diagnostics - Checks Mac Python Interpreter', () => {
expect(messagePrompt).not.be.equal(undefined, 'Message prompt not set');
expect(messagePrompt!.commandPrompts).to.be.deep.equal([
{ prompt: 'Select Python Interpreter', command: cmd },
{ prompt: 'Do not show again', command: cmdIgnore },
{ prompt: "Don't show again", command: cmdIgnore },
]);
});
test('Should not display a message if No Interpreters diagnostic has been ignored', async () => {
Expand Down
12 changes: 6 additions & 6 deletions src/test/common/installer/installer.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,10 @@ suite('Module Installer only', () => {
TypeMoq.It.isAnyString(),
TypeMoq.It.isValue('Install'),
TypeMoq.It.isValue('Select Linter'),
TypeMoq.It.isValue('Do not show again'),
TypeMoq.It.isValue("Don't show again"),
),
)
.returns(async () => 'Do not show again')
.returns(async () => "Don't show again")
.verifiable(TypeMoq.Times.once());
const persistVal = TypeMoq.Mock.ofType<IPersistentState<boolean>>();
let mockPersistVal = false;
Expand Down Expand Up @@ -367,7 +367,7 @@ suite('Module Installer only', () => {
TypeMoq.It.isAnyString(),
TypeMoq.It.isValue('Install'),
TypeMoq.It.isValue('Select Linter'),
TypeMoq.It.isValue('Do not show again'),
TypeMoq.It.isValue("Don't show again"),
),
)
.returns(async () => undefined)
Expand Down Expand Up @@ -864,7 +864,7 @@ suite('Module Installer only', () => {

test('Ensure 3 options for pylint', async () => {
const product = Product.pylint;
const options = ['Select Linter', 'Do not show again'];
const options = ['Select Linter', "Don't show again"];
const productName = ProductNames.get(product)!;

await installer.promptToInstallImplementation(product, resource);
Expand All @@ -875,7 +875,7 @@ suite('Module Installer only', () => {
});
test('Ensure select linter command is invoked', async () => {
const product = Product.pylint;
const options = ['Select Linter', 'Do not show again'];
const options = ['Select Linter', "Don't show again"];
const productName = ProductNames.get(product)!;
when(
appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]),
Expand All @@ -892,7 +892,7 @@ suite('Module Installer only', () => {
});
test('If install button is selected, install linter and return response', async () => {
const product = Product.pylint;
const options = ['Select Linter', 'Do not show again'];
const options = ['Select Linter', "Don't show again"];
const productName = ProductNames.get(product)!;
when(
appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1]),
Expand Down
Loading
Loading