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 when deactivate command is run in terminal #22133

Merged
merged 12 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"quickPickSortByLabel",
"testObserver",
"quickPickItemTooltip",
"saveEditor"
"saveEditor",
"terminalDataWriteEvent"
],
"author": {
"name": "Microsoft Corporation"
Expand Down
12 changes: 11 additions & 1 deletion src/client/common/application/applicationShell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
DocumentSelector,
env,
Event,
EventEmitter,
InputBox,
InputBoxOptions,
languages,
Expand Down Expand Up @@ -37,7 +38,8 @@ import {
WorkspaceFolder,
WorkspaceFolderPickOptions,
} from 'vscode';
import { IApplicationShell } from './types';
import { traceError } from '../../logging';
import { IApplicationShell, TerminalDataWriteEvent } from './types';

@injectable()
export class ApplicationShell implements IApplicationShell {
Expand Down Expand Up @@ -172,4 +174,12 @@ export class ApplicationShell implements IApplicationShell {
public createLanguageStatusItem(id: string, selector: DocumentSelector): LanguageStatusItem {
return languages.createLanguageStatusItem(id, selector);
}
public get onDidWriteTerminalData(): Event<TerminalDataWriteEvent> {
try {
return window.onDidWriteTerminalData;
} catch (ex) {
traceError('Failed to get proposed API onDidWriteTerminalData', ex);
return new EventEmitter<TerminalDataWriteEvent>().event;
}
}
}
18 changes: 18 additions & 0 deletions src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ import { Resource } from '../types';
import { ICommandNameArgumentTypeMapping } from './commands';
import { ExtensionContextKey } from './contextKeys';

export interface TerminalDataWriteEvent {
/**
* The {@link Terminal} for which the data was written.
*/
readonly terminal: Terminal;
/**
* The data being written.
*/
readonly data: string;
}

export const IApplicationShell = Symbol('IApplicationShell');
export interface IApplicationShell {
/**
Expand All @@ -75,6 +86,13 @@ export interface IApplicationShell {
*/
readonly onDidChangeWindowState: Event<WindowState>;

/**
* An event which fires when the terminal's child pseudo-device is written to (the shell).
* In other words, this provides access to the raw data stream from the process running
* within the terminal, including VT sequences.
*/
readonly onDidWriteTerminalData: Event<TerminalDataWriteEvent>;

showInformationMessage(message: string, ...items: string[]): Thenable<string | undefined>;

/**
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ export namespace Interpreters {
export const terminalEnvVarCollectionPrompt = l10n.t(
'The Python extension automatically activates all terminals using the selected environment, even when the name of the environment{0} is not present in the terminal prompt. [Learn more](https://aka.ms/vscodePythonTerminalActivation).',
);
export const terminalDeactivatePrompt = l10n.t(
'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved with a few simple steps.',
);
export const deactivateDoneButton = l10n.t('Done, it works');
export const activatedCondaEnvLaunch = l10n.t(
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
);
Expand Down
8 changes: 0 additions & 8 deletions src/client/interpreter/activation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,3 @@ 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;
}
13 changes: 1 addition & 12 deletions src/client/interpreter/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
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, ITerminalEnvVarCollectionService } from './activation/types';
import { IEnvironmentActivationService } from './activation/types';
import { InterpreterAutoSelectionService } from './autoSelection/index';
import { InterpreterAutoSelectionProxyService } from './autoSelection/proxy';
import { IInterpreterAutoSelectionService, IInterpreterAutoSelectionProxyService } from './autoSelection/types';
Expand Down Expand Up @@ -110,13 +108,4 @@ export function registerTypes(serviceManager: IServiceManager): void {
IEnvironmentActivationService,
EnvironmentActivationService,
);
serviceManager.addSingleton<ITerminalEnvVarCollectionService>(
ITerminalEnvVarCollectionService,
TerminalEnvVarCollectionService,
);
serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
TerminalEnvVarCollectionPrompt,
);
}
1 change: 1 addition & 0 deletions src/client/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export enum EventName {
TERMINAL_SHELL_IDENTIFICATION = 'TERMINAL_SHELL_IDENTIFICATION',
PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT = 'PYTHON_INTERPRETER_ACTIVATE_ENVIRONMENT_PROMPT',
PYTHON_NOT_INSTALLED_PROMPT = 'PYTHON_NOT_INSTALLED_PROMPT',
TERMINAL_DEACTIVATE_PROMPT = 'TERMINAL_DEACTIVATE_PROMPT',
CONDA_INHERIT_ENV_PROMPT = 'CONDA_INHERIT_ENV_PROMPT',
REQUIRE_JUPYTER_PROMPT = 'REQUIRE_JUPYTER_PROMPT',
ACTIVATED_CONDA_ENV_LAUNCH = 'ACTIVATED_CONDA_ENV_LAUNCH',
Expand Down
18 changes: 18 additions & 0 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,24 @@ export interface IEventNamePropertyMapping {
*/
selection: 'Allow' | 'Close' | undefined;
};
/**
* Telemetry event sent with details when user clicks the prompt with the following message:
*
* 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved with a few simple steps.'
*/
/* __GDPR__
"terminal_deactivate_prompt" : {
"selection" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "karrtikr" }
}
*/
[EventName.TERMINAL_DEACTIVATE_PROMPT]: {
/**
* `See Instructions` When 'See Instructions' option is selected
* `Done, it works` When 'Done, it works' option is selected
* `Don't show again` When 'Don't show again' option is selected
*/
selection: 'See Instructions' | 'Done, it works' | "Don't show again" | undefined;
};
/**
* Telemetry event sent with details when user attempts to run in interactive window when Jupyter is not installed.
*/
Expand Down
91 changes: 91 additions & 0 deletions src/client/terminals/envCollectionActivation/deactivatePrompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { IApplicationEnvironment, IApplicationShell } from '../../common/application/types';
import { IBrowserService, IDisposableRegistry, IExperimentService, IPersistentStateFactory } from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
import { IInterpreterService } from '../../interpreter/contracts';
import { PythonEnvType } from '../../pythonEnvironments/base/info';
import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors/baseShellDetector';
import { TerminalShellType } from '../../common/terminal/types';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';

export const terminalDeactivationPromptKey = 'TERMINAL_DEACTIVATION_PROMPT_KEY';

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

constructor(
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IBrowserService) private readonly browserService: IBrowserService,
@inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {}

public async activate(): Promise<void> {
if (!inTerminalEnvVarExperiment(this.experimentService)) {
return;
}
this.disposableRegistry.push(
this.appShell.onDidWriteTerminalData(async (e) => {
if (!e.data.includes('deactivate')) {
return;
}
const shellType = identifyShellFromShellPath(this.appEnvironment.shell);
if (shellType === TerminalShellType.commandPrompt) {
return;
}
const { terminal } = e;
const cwd =
'cwd' in terminal.creationOptions && terminal.creationOptions.cwd
? terminal.creationOptions.cwd
: undefined;
const resource = typeof cwd === 'string' ? Uri.file(cwd) : cwd;
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
if (interpreter?.type !== PythonEnvType.Virtual) {
return;
}
await this.notifyUsers();
}),
);
}

private async notifyUsers(): Promise<void> {
const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(
terminalDeactivationPromptKey,
true,
);
if (!notificationPromptEnabled.value) {
return;
}
const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain];
const telemetrySelections: ['See Instructions', 'Done, it works', "Don't show again"] = [
'See Instructions',
'Done, it works',
"Don't show again",
];
const selection = await this.appShell.showWarningMessage(Interpreters.terminalDeactivatePrompt, ...prompts);
if (!selection) {
return;
}
sendTelemetryEvent(EventName.TERMINAL_DEACTIVATE_PROMPT, undefined, {
selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined,
});
if (selection === prompts[0]) {
const url = `https://aka.ms/AAmx2ft`;
this.browserService.launch(url);
}
if (selection === prompts[1] || selection === prompts[2]) {
await notificationPromptEnabled.updateValue(false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import {
} 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';
import { IInterpreterService } from '../contracts';
import { IInterpreterService } from '../../interpreter/contracts';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { ITerminalEnvVarCollectionService } from '../types';

export const terminalEnvCollectionPromptKey = 'TERMINAL_ENV_COLLECTION_PROMPT_KEY';

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

constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ import {
import { Deferred, createDeferred } from '../../common/utils/async';
import { Interpreters } from '../../common/utils/localize';
import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
import { IInterpreterService } from '../contracts';
import { defaultShells } from './service';
import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
import { IInterpreterService } from '../../interpreter/contracts';
import { defaultShells } from '../../interpreter/activation/service';
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
import { EnvironmentType, PythonEnvironment } 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';
import { normCase } from '../../common/platform/fs-paths';
import { PythonEnvType } from '../../pythonEnvironments/base/info';
import { ITerminalEnvVarCollectionService } from '../types';

@injectable()
export class TerminalEnvVarCollectionService implements IExtensionActivationService, ITerminalEnvVarCollectionService {
Expand Down
38 changes: 26 additions & 12 deletions src/client/terminals/serviceRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { interfaces } from 'inversify';
import { ClassType } from '../ioc/types';
import { IServiceManager } from '../ioc/types';
import { TerminalAutoActivation } from './activation';
import { CodeExecutionManager } from './codeExecution/codeExecutionManager';
import { DjangoShellCodeExecutionProvider } from './codeExecution/djangoShellCodeExecution';
import { CodeExecutionHelper } from './codeExecution/helper';
import { ReplProvider } from './codeExecution/repl';
import { TerminalCodeExecutionProvider } from './codeExecution/terminalCodeExecution';
import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, ITerminalAutoActivation } from './types';
import {
ICodeExecutionHelper,
ICodeExecutionManager,
ICodeExecutionService,
ITerminalAutoActivation,
ITerminalEnvVarCollectionService,
} from './types';
import { TerminalEnvVarCollectionService } from './envCollectionActivation/service';
import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types';
import { TerminalDeactivateLimitationPrompt } from './envCollectionActivation/deactivatePrompt';
import { TerminalIndicatorPrompt } from './envCollectionActivation/indicatorPrompt';

interface IServiceRegistry {
addSingleton<T>(
serviceIdentifier: interfaces.ServiceIdentifier<T>,
constructor: ClassType<T>,
name?: string | number | symbol,
): void;
}

export function registerTypes(serviceManager: IServiceRegistry): void {
export function registerTypes(serviceManager: IServiceManager): void {
serviceManager.addSingleton<ICodeExecutionHelper>(ICodeExecutionHelper, CodeExecutionHelper);

serviceManager.addSingleton<ICodeExecutionManager>(ICodeExecutionManager, CodeExecutionManager);
Expand All @@ -37,4 +38,17 @@ export function registerTypes(serviceManager: IServiceRegistry): void {
serviceManager.addSingleton<ICodeExecutionService>(ICodeExecutionService, ReplProvider, 'repl');

serviceManager.addSingleton<ITerminalAutoActivation>(ITerminalAutoActivation, TerminalAutoActivation);
serviceManager.addSingleton<ITerminalEnvVarCollectionService>(
ITerminalEnvVarCollectionService,
TerminalEnvVarCollectionService,
);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
TerminalIndicatorPrompt,
);
serviceManager.addSingleton<IExtensionSingleActivationService>(
IExtensionSingleActivationService,
TerminalDeactivateLimitationPrompt,
);
serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
}
8 changes: 8 additions & 0 deletions src/client/terminals/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,11 @@ export interface ITerminalAutoActivation extends IDisposable {
register(): void;
disableAutoActivation(terminal: Terminal): void;
}

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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ import {
IPersistentStateFactory,
IPythonSettings,
} from '../../../client/common/types';
import { TerminalEnvVarCollectionPrompt } from '../../../client/interpreter/activation/terminalEnvVarCollectionPrompt';
import { ITerminalEnvVarCollectionService } from '../../../client/interpreter/activation/types';
import { TerminalIndicatorPrompt } from '../../../client/terminals/envCollectionActivation/indicatorPrompt';
import { Common, Interpreters } from '../../../client/common/utils/localize';
import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups';
import { sleep } from '../../core';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
import { ITerminalEnvVarCollectionService } from '../../../client/terminals/types';

suite('Terminal Environment Variable Collection Prompt', () => {
let shell: IApplicationShell;
Expand All @@ -28,7 +28,7 @@ suite('Terminal Environment Variable Collection Prompt', () => {
let activeResourceService: IActiveResourceService;
let terminalEnvVarCollectionService: ITerminalEnvVarCollectionService;
let persistentStateFactory: IPersistentStateFactory;
let terminalEnvVarCollectionPrompt: TerminalEnvVarCollectionPrompt;
let terminalEnvVarCollectionPrompt: TerminalIndicatorPrompt;
let terminalEventEmitter: EventEmitter<Terminal>;
let notificationEnabled: IPersistentState<boolean>;
let configurationService: IConfigurationService;
Expand Down Expand Up @@ -61,7 +61,7 @@ suite('Terminal Environment Variable Collection Prompt', () => {
);
when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
when(terminalManager.onDidOpenTerminal).thenReturn(terminalEventEmitter.event);
terminalEnvVarCollectionPrompt = new TerminalEnvVarCollectionPrompt(
terminalEnvVarCollectionPrompt = new TerminalIndicatorPrompt(
instance(shell),
instance(persistentStateFactory),
instance(terminalManager),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
import { Interpreters } from '../../../client/common/utils/localize';
import { OSType, getOSType } from '../../../client/common/utils/platform';
import { defaultShells } from '../../../client/interpreter/activation/service';
import { TerminalEnvVarCollectionService } from '../../../client/interpreter/activation/terminalEnvVarCollectionService';
import { TerminalEnvVarCollectionService } from '../../../client/terminals/envCollectionActivation/service';
import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types';
import { IInterpreterService } from '../../../client/interpreter/contracts';
import { PathUtils } from '../../../client/common/platform/pathUtils';
Expand Down
Loading
Loading