From ce4da13bea363de582d341759532381f519b92e2 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 13 Oct 2023 19:50:41 +0000 Subject: [PATCH 01/38] Intial commit --- package.json | 3 +- .../common/application/applicationShell.ts | 12 +- src/client/common/application/types.ts | 18 ++ src/client/common/utils/localize.ts | 4 + src/client/interpreter/activation/types.ts | 8 - src/client/interpreter/serviceRegistry.ts | 13 +- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 18 ++ .../deactivatePrompt.ts | 91 +++++++ .../indicatorPrompt.ts} | 6 +- .../envCollectionActivation/service.ts} | 7 +- src/client/terminals/serviceRegistry.ts | 38 ++- src/client/terminals/types.ts | 8 + ...erminalEnvVarCollectionPrompt.unit.test.ts | 8 +- ...rminalEnvVarCollectionService.unit.test.ts | 2 +- .../deactivatePrompt.unit.test.ts | 251 ++++++++++++++++++ .../terminals/serviceRegistry.unit.test.ts | 16 ++ ...scode.proposed.terminalDataWriteEvent.d.ts | 31 +++ 18 files changed, 490 insertions(+), 45 deletions(-) create mode 100644 src/client/terminals/envCollectionActivation/deactivatePrompt.ts rename src/client/{interpreter/activation/terminalEnvVarCollectionPrompt.ts => terminals/envCollectionActivation/indicatorPrompt.ts} (95%) rename src/client/{interpreter/activation/terminalEnvVarCollectionService.ts => terminals/envCollectionActivation/service.ts} (98%) create mode 100644 src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts create mode 100644 typings/vscode-proposed/vscode.proposed.terminalDataWriteEvent.d.ts diff --git a/package.json b/package.json index 20401cc43762..1748442cdd33 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "quickPickSortByLabel", "testObserver", "quickPickItemTooltip", - "saveEditor" + "saveEditor", + "terminalDataWriteEvent" ], "author": { "name": "Microsoft Corporation" diff --git a/src/client/common/application/applicationShell.ts b/src/client/common/application/applicationShell.ts index 454662472010..aadf80186900 100644 --- a/src/client/common/application/applicationShell.ts +++ b/src/client/common/application/applicationShell.ts @@ -10,6 +10,7 @@ import { DocumentSelector, env, Event, + EventEmitter, InputBox, InputBoxOptions, languages, @@ -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 { @@ -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 { + try { + return window.onDidWriteTerminalData; + } catch (ex) { + traceError('Failed to get proposed API onDidWriteTerminalData', ex); + return new EventEmitter().event; + } + } } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index fa2ced6c45da..863f5e4651b2 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -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 { /** @@ -75,6 +86,13 @@ export interface IApplicationShell { */ readonly onDidChangeWindowState: Event; + /** + * 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; + showInformationMessage(message: string, ...items: string[]): Thenable; /** diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index bbb55a79ce40..5fdc8602009f 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -198,6 +198,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?', ); diff --git a/src/client/interpreter/activation/types.ts b/src/client/interpreter/activation/types.ts index 2b364cbeb862..e00ef9b62b3f 100644 --- a/src/client/interpreter/activation/types.ts +++ b/src/client/interpreter/activation/types.ts @@ -21,11 +21,3 @@ export interface IEnvironmentActivationService { interpreter?: PythonEnvironment, ): Promise; } - -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; -} diff --git a/src/client/interpreter/serviceRegistry.ts b/src/client/interpreter/serviceRegistry.ts index 018e7abfdc46..422776bd5e43 100644 --- a/src/client/interpreter/serviceRegistry.ts +++ b/src/client/interpreter/serviceRegistry.ts @@ -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'; @@ -110,13 +108,4 @@ export function registerTypes(serviceManager: IServiceManager): void { IEnvironmentActivationService, EnvironmentActivationService, ); - serviceManager.addSingleton( - ITerminalEnvVarCollectionService, - TerminalEnvVarCollectionService, - ); - serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService); - serviceManager.addSingleton( - IExtensionSingleActivationService, - TerminalEnvVarCollectionPrompt, - ); } diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 301502a0f6fa..bd7d8b953c12 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -27,6 +27,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', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index f69da6046254..4c612b5b86bc 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1291,6 +1291,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. */ diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts new file mode 100644 index 000000000000..460144303f18 --- /dev/null +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -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 { + 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 { + 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); + } + } +} diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionPrompt.ts b/src/client/terminals/envCollectionActivation/indicatorPrompt.ts similarity index 95% rename from src/client/interpreter/activation/terminalEnvVarCollectionPrompt.ts rename to src/client/terminals/envCollectionActivation/indicatorPrompt.ts index c8aea205a32a..bf648eefe8e9 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionPrompt.ts +++ b/src/client/terminals/envCollectionActivation/indicatorPrompt.ts @@ -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( diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/terminals/envCollectionActivation/service.ts similarity index 98% rename from src/client/interpreter/activation/terminalEnvVarCollectionService.ts rename to src/client/terminals/envCollectionActivation/service.ts index 92e97c95e468..4b461271f4a5 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -28,9 +28,9 @@ 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'; @@ -38,6 +38,7 @@ 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 { diff --git a/src/client/terminals/serviceRegistry.ts b/src/client/terminals/serviceRegistry.ts index a39ef31a8fe4..a9da776d011a 100644 --- a/src/client/terminals/serviceRegistry.ts +++ b/src/client/terminals/serviceRegistry.ts @@ -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( - serviceIdentifier: interfaces.ServiceIdentifier, - constructor: ClassType, - name?: string | number | symbol, - ): void; -} - -export function registerTypes(serviceManager: IServiceRegistry): void { +export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(ICodeExecutionHelper, CodeExecutionHelper); serviceManager.addSingleton(ICodeExecutionManager, CodeExecutionManager); @@ -37,4 +38,17 @@ export function registerTypes(serviceManager: IServiceRegistry): void { serviceManager.addSingleton(ICodeExecutionService, ReplProvider, 'repl'); serviceManager.addSingleton(ITerminalAutoActivation, TerminalAutoActivation); + serviceManager.addSingleton( + ITerminalEnvVarCollectionService, + TerminalEnvVarCollectionService, + ); + serviceManager.addSingleton( + IExtensionSingleActivationService, + TerminalIndicatorPrompt, + ); + serviceManager.addSingleton( + IExtensionSingleActivationService, + TerminalDeactivateLimitationPrompt, + ); + serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService); } diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 48e39d4e1c81..ba30b8f6d47d 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -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; +} diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts index baa83c8b11c5..5d4da49ebb45 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionPrompt.unit.test.ts @@ -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; @@ -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; let notificationEnabled: IPersistentState; let configurationService: IConfigurationService; @@ -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), diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index e41d6ce4d53c..5e572e7ad06f 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -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'; diff --git a/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts b/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts new file mode 100644 index 000000000000..acd8ee99e5d7 --- /dev/null +++ b/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { mock, when, anything, instance, verify, reset } from 'ts-mockito'; +import { EventEmitter, Terminal, TerminalDataWriteEvent, Uri } from 'vscode'; +import { IApplicationEnvironment, IApplicationShell } from '../../../client/common/application/types'; +import { + IBrowserService, + IExperimentService, + IPersistentState, + IPersistentStateFactory, +} from '../../../client/common/types'; +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 { TerminalDeactivateLimitationPrompt } from '../../../client/terminals/envCollectionActivation/deactivatePrompt'; +import { PythonEnvType } from '../../../client/pythonEnvironments/base/info'; +import { TerminalShellType } from '../../../client/common/terminal/types'; + +suite('Terminal Deactivation Limitation Prompt', () => { + let shell: IApplicationShell; + let experimentService: IExperimentService; + let persistentStateFactory: IPersistentStateFactory; + let appEnvironment: IApplicationEnvironment; + let deactivatePrompt: TerminalDeactivateLimitationPrompt; + let terminalWriteEvent: EventEmitter; + let notificationEnabled: IPersistentState; + let browserService: IBrowserService; + let interpreterService: IInterpreterService; + const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain]; + const expectedMessage = Interpreters.terminalDeactivatePrompt; + + setup(async () => { + shell = mock(); + interpreterService = mock(); + experimentService = mock(); + persistentStateFactory = mock(); + appEnvironment = mock(); + when(appEnvironment.shell).thenReturn('bash'); + browserService = mock(); + notificationEnabled = mock>(); + terminalWriteEvent = new EventEmitter(); + when(persistentStateFactory.createGlobalPersistentState(anything(), true)).thenReturn( + instance(notificationEnabled), + ); + when(shell.onDidWriteTerminalData).thenReturn(terminalWriteEvent.event); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); + deactivatePrompt = new TerminalDeactivateLimitationPrompt( + instance(shell), + instance(persistentStateFactory), + [], + instance(interpreterService), + instance(browserService), + instance(appEnvironment), + instance(experimentService), + ); + }); + + test('Show notification when "deactivate" command is run when a virtual env is selected', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).once(); + }); + + test('When using cmd, do not show notification for the same', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + reset(appEnvironment); + when(appEnvironment.shell).thenReturn(TerminalShellType.commandPrompt); + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).once(); + }); + + test('When not in experiment, do not show notification for the same', async () => { + reset(experimentService); + when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).never(); + }); + + test('Do not show notification if notification is disabled', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(false); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).never(); + }); + + test('Do not show notification when virtual env is not activated for terminal', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Conda, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).never(); + }); + + test("Disable notification if `Don't show again` is clicked", async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn(Promise.resolve(Common.doNotShowAgain)); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(notificationEnabled.updateValue(false)).once(); + }); + + test('Disable notification if `Done, it works` is clicked', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn( + Promise.resolve(Interpreters.deactivateDoneButton), + ); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(notificationEnabled.updateValue(false)).once(); + }); + + test('Open link to workaround if `See instructions` is clicked', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn(Promise.resolve(Common.seeInstructions)); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).once(); + verify(browserService.launch(anything())).once(); + }); + + test('Do not perform any action if prompt is closed', async () => { + const resource = Uri.file('a'); + const terminal = ({ + creationOptions: { + cwd: resource, + }, + } as unknown) as Terminal; + when(notificationEnabled.value).thenReturn(true); + when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ + type: PythonEnvType.Virtual, + } as unknown) as PythonEnvironment); + when(shell.showWarningMessage(expectedMessage, ...prompts)).thenResolve(undefined); + + await deactivatePrompt.activate(); + terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); + await sleep(1); + + verify(shell.showWarningMessage(expectedMessage, ...prompts)).once(); + verify(notificationEnabled.updateValue(false)).never(); + verify(browserService.launch(anything())).never(); + }); +}); diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index 38a9a9744e91..816afa17cf88 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as typemoq from 'typemoq'; +import { IExtensionActivationService, IExtensionSingleActivationService } from '../../client/activation/types'; import { IServiceManager } from '../../client/ioc/types'; import { TerminalAutoActivation } from '../../client/terminals/activation'; import { CodeExecutionManager } from '../../client/terminals/codeExecution/codeExecutionManager'; @@ -9,12 +10,16 @@ import { DjangoShellCodeExecutionProvider } from '../../client/terminals/codeExe import { CodeExecutionHelper } from '../../client/terminals/codeExecution/helper'; import { ReplProvider } from '../../client/terminals/codeExecution/repl'; import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecution/terminalCodeExecution'; +import { TerminalDeactivateLimitationPrompt } from '../../client/terminals/envCollectionActivation/deactivatePrompt'; +import { TerminalIndicatorPrompt } from '../../client/terminals/envCollectionActivation/indicatorPrompt'; +import { TerminalEnvVarCollectionService } from '../../client/terminals/envCollectionActivation/service'; import { registerTypes } from '../../client/terminals/serviceRegistry'; import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, ITerminalAutoActivation, + ITerminalEnvVarCollectionService, } from '../../client/terminals/types'; suite('Terminal - Service Registry', () => { @@ -27,6 +32,9 @@ suite('Terminal - Service Registry', () => { [ICodeExecutionService, ReplProvider, 'repl'], [ITerminalAutoActivation, TerminalAutoActivation], [ICodeExecutionService, TerminalCodeExecutionProvider, 'standard'], + [ITerminalEnvVarCollectionService, TerminalEnvVarCollectionService], + [IExtensionSingleActivationService, TerminalIndicatorPrompt], + [IExtensionSingleActivationService, TerminalDeactivateLimitationPrompt], ].forEach((args) => { if (args.length === 2) { services @@ -50,6 +58,14 @@ suite('Terminal - Service Registry', () => { .verifiable(typemoq.Times.once()); } }); + services + .setup((s) => + s.addBinding( + typemoq.It.is((v) => ITerminalEnvVarCollectionService === v), + typemoq.It.is((value) => IExtensionActivationService === value), + ), + ) + .verifiable(typemoq.Times.once()); registerTypes(services.object); diff --git a/typings/vscode-proposed/vscode.proposed.terminalDataWriteEvent.d.ts b/typings/vscode-proposed/vscode.proposed.terminalDataWriteEvent.d.ts new file mode 100644 index 000000000000..6913b862c70f --- /dev/null +++ b/typings/vscode-proposed/vscode.proposed.terminalDataWriteEvent.d.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + // https://github.com/microsoft/vscode/issues/78502 + // + // This API is still proposed but we don't intent on promoting it to stable due to problems + // around performance. See #145234 for a more likely API to get stabilized. + + export interface TerminalDataWriteEvent { + /** + * The {@link Terminal} for which the data was written. + */ + readonly terminal: Terminal; + /** + * The data being written. + */ + readonly data: string; + } + + namespace window { + /** + * 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. + */ + export const onDidWriteTerminalData: Event; + } +} From 70b6fcd01869d05426bf3fa3619decfecb153f71 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 04:53:02 +0000 Subject: [PATCH 02/38] Do not prepend twice --- .../envCollectionActivation/service.ts | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 4b461271f4a5..0038511a3dbb 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -10,6 +10,7 @@ import { WorkspaceFolder, GlobalEnvironmentVariableCollection, EnvironmentVariableScope, + EnvironmentVariableMutatorOptions, } from 'vscode'; import { pathExists } from 'fs-extra'; import { IExtensionActivationService } from '../../activation/types'; @@ -172,6 +173,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 in some cases is a shell variable (not an env variable) so "env" might not contain it, calculate it in that case. env.PS1 = await this.getPS1(shell, resource, env); + const prependOptions = this.getPrependOptions(); // Clear any previously set env vars from collection envVarCollection.clear(); @@ -186,10 +188,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ 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, - applyAtProcessCreation: false, - }); + envVarCollection.prepend(key, value, prependOptions); return; } if (key === 'PATH') { @@ -199,19 +198,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const prependedPart = env.PATH.slice(0, -processEnv.PATH.length); value = prependedPart; traceVerbose(`Prepending environment variable ${key} in collection with ${value}`); - envVarCollection.prepend(key, value, { - applyAtShellIntegration: true, - applyAtProcessCreation: true, - }); + envVarCollection.prepend(key, value, prependOptions); } else { if (!value.endsWith(this.separator)) { value = value.concat(this.separator); } traceVerbose(`Prepending environment variable ${key} in collection to ${value}`); - envVarCollection.prepend(key, value, { - applyAtShellIntegration: true, - applyAtProcessCreation: true, - }); + envVarCollection.prepend(key, value, prependOptions); } return; } @@ -330,6 +323,22 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } } + private getPrependOptions(): EnvironmentVariableMutatorOptions { + const value = this.workspaceService + .getConfiguration('terminal') + .get('integrated.shellIntegration.enabled'); + // Make sure to prepend either at shell integration or process creation, not both. + return value + ? { + applyAtShellIntegration: true, + applyAtProcessCreation: false, + } + : { + applyAtShellIntegration: false, + applyAtProcessCreation: true, + }; + } + private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) { const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection; return envVarCollection.getScoped(scope); From c839ca9f931e149fff4d0bcc312efda9e92f7ffb Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 04:53:24 +0000 Subject: [PATCH 03/38] Add deactivate prompt --- pythonFiles/deactivate | 33 +++++++ pythonFiles/deactivate.csh | 6 ++ pythonFiles/deactivate.fish | 36 ++++++++ pythonFiles/deactivate.ps1 | 31 +++++++ src/client/common/platform/fileSystem.ts | 3 + src/client/common/utils/localize.ts | 3 + .../deactivatePrompt.ts | 85 +++++++++++++++++-- .../deactivateScripts.ts | 70 +++++++++++++++ 8 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 pythonFiles/deactivate create mode 100644 pythonFiles/deactivate.csh create mode 100644 pythonFiles/deactivate.fish create mode 100644 pythonFiles/deactivate.ps1 create mode 100644 src/client/terminals/envCollectionActivation/deactivateScripts.ts diff --git a/pythonFiles/deactivate b/pythonFiles/deactivate new file mode 100644 index 000000000000..6ede3da311a9 --- /dev/null +++ b/pythonFiles/deactivate @@ -0,0 +1,33 @@ +# Same as deactivate in "/bin/activate" +deactivate () { + if [ -n "${_OLD_VIRTUAL_PATH:-}" ] ; then + PATH="${_OLD_VIRTUAL_PATH:-}" + export PATH + unset _OLD_VIRTUAL_PATH + fi + if [ -n "${_OLD_VIRTUAL_PYTHONHOME:-}" ] ; then + PYTHONHOME="${_OLD_VIRTUAL_PYTHONHOME:-}" + export PYTHONHOME + unset _OLD_VIRTUAL_PYTHONHOME + fi + if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then + hash -r 2> /dev/null + fi + if [ -n "${_OLD_VIRTUAL_PS1:-}" ] ; then + PS1="${_OLD_VIRTUAL_PS1:-}" + export PS1 + unset _OLD_VIRTUAL_PS1 + fi + unset VIRTUAL_ENV + unset VIRTUAL_ENV_PROMPT + if [ ! "${1:-}" = "nondestructive" ] ; then + unset -f deactivate + fi +} + +# Initialize the variables required by deactivate function +_OLD_VIRTUAL_PS1="${PS1:-}" +_OLD_VIRTUAL_PATH="$PATH" +if [ -n "${PYTHONHOME:-}" ] ; then + _OLD_VIRTUAL_PYTHONHOME="${PYTHONHOME:-}" +fi diff --git a/pythonFiles/deactivate.csh b/pythonFiles/deactivate.csh new file mode 100644 index 000000000000..ef4d0d393897 --- /dev/null +++ b/pythonFiles/deactivate.csh @@ -0,0 +1,6 @@ +# Same as deactivate in "/bin/activate.csh" +alias deactivate 'test $?_OLD_VIRTUAL_PATH != 0 && setenv PATH "$_OLD_VIRTUAL_PATH" && unset _OLD_VIRTUAL_PATH; rehash; test $?_OLD_VIRTUAL_PROMPT != 0 && set prompt="$_OLD_VIRTUAL_PROMPT" && unset _OLD_VIRTUAL_PROMPT; unsetenv VIRTUAL_ENV; unsetenv VIRTUAL_ENV_PROMPT; test "\!:*" != "nondestructive" && unalias deactivate' + +# Initialize the variables required by deactivate function +set _OLD_VIRTUAL_PROMPT="$prompt" +set _OLD_VIRTUAL_PATH="$PATH" diff --git a/pythonFiles/deactivate.fish b/pythonFiles/deactivate.fish new file mode 100644 index 000000000000..6e8dc26db81e --- /dev/null +++ b/pythonFiles/deactivate.fish @@ -0,0 +1,36 @@ +# Same as deactivate in "/bin/activate.fish" +function deactivate -d "Exit virtual environment and return to normal shell environment" + # reset old environment variables + if test -n "$_OLD_VIRTUAL_PATH" + set -gx PATH $_OLD_VIRTUAL_PATH + set -e _OLD_VIRTUAL_PATH + end + if test -n "$_OLD_VIRTUAL_PYTHONHOME" + set -gx PYTHONHOME $_OLD_VIRTUAL_PYTHONHOME + set -e _OLD_VIRTUAL_PYTHONHOME + end + + if test -n "$_OLD_FISH_PROMPT_OVERRIDE" + set -e _OLD_FISH_PROMPT_OVERRIDE + if functions -q _old_fish_prompt + functions -e fish_prompt + functions -c _old_fish_prompt fish_prompt + functions -e _old_fish_prompt + end + end + + set -e VIRTUAL_ENV + set -e VIRTUAL_ENV_PROMPT + if test "$argv[1]" != "nondestructive" + functions -e deactivate + end +end + +# Initialize the variables required by deactivate function +set -gx _OLD_VIRTUAL_PATH $PATH +if test -z "$VIRTUAL_ENV_DISABLE_PROMPT" + functions -c fish_prompt _old_fish_prompt +end +if set -q PYTHONHOME + set -gx _OLD_VIRTUAL_PYTHONHOME $PYTHONHOME +end diff --git a/pythonFiles/deactivate.ps1 b/pythonFiles/deactivate.ps1 new file mode 100644 index 000000000000..65dd80907d90 --- /dev/null +++ b/pythonFiles/deactivate.ps1 @@ -0,0 +1,31 @@ +# Same as deactivate in "Activate.ps1" +function global:deactivate ([switch]$NonDestructive) { + if (Test-Path function:_OLD_VIRTUAL_PROMPT) { + copy-item function:_OLD_VIRTUAL_PROMPT function:prompt + remove-item function:_OLD_VIRTUAL_PROMPT + } + if (Test-Path env:_OLD_VIRTUAL_PYTHONHOME) { + copy-item env:_OLD_VIRTUAL_PYTHONHOME env:PYTHONHOME + remove-item env:_OLD_VIRTUAL_PYTHONHOME + } + if (Test-Path env:_OLD_VIRTUAL_PATH) { + copy-item env:_OLD_VIRTUAL_PATH env:PATH + remove-item env:_OLD_VIRTUAL_PATH + } + if (Test-Path env:VIRTUAL_ENV) { + remove-item env:VIRTUAL_ENV + } + if (!$NonDestructive) { + remove-item function:deactivate + } +} + +# Initialize the variables required by deactivate function +if (! $env:VIRTUAL_ENV_DISABLE_PROMPT) { + function global:_OLD_VIRTUAL_PROMPT {""} + copy-item function:prompt function:_OLD_VIRTUAL_PROMPT +} +if (Test-Path env:PYTHONHOME) { + copy-item env:PYTHONHOME env:_OLD_VIRTUAL_PYTHONHOME +} +copy-item env:PATH env:_OLD_VIRTUAL_PATH diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 8f962b0f776f..8da060c10c25 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -209,6 +209,9 @@ export class RawFileSystem implements IRawFileSystem { public async copyFile(src: string, dest: string): Promise { const srcURI = vscode.Uri.file(src); const destURI = vscode.Uri.file(dest); + if (!(await this.pathExists(this.paths.dirname(dest)))) { + await this.mkdirp(this.paths.dirname(dest)); + } // The VS Code API will automatically create the target parent // directory if it does not exist (even though the docs imply // otherwise). So we have to manually stat, just to be sure. diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 5fdc8602009f..1fc6186a9fa2 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -201,6 +201,9 @@ export namespace Interpreters { 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 terminalDeactivateShellSpecificPrompt = l10n.t( + 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved by appending a line to "{0}" and restarting the shell. [Learn more](https://aka.ms/AAmx2ft).', + ); 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?', diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 460144303f18..6856af816dd3 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -2,9 +2,16 @@ // 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 { Position, TextDocument, Uri, window, workspace, WorkspaceEdit, Range, TextEditorRevealType } from 'vscode'; +import * as path from 'path'; +import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types'; +import { + IBrowserService, + IDisposableRegistry, + IExperimentService, + IPersistentState, + IPersistentStateFactory, +} from '../../common/types'; import { Common, Interpreters } from '../../common/utils/localize'; import { IExtensionSingleActivationService } from '../../activation/types'; import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; @@ -12,15 +19,21 @@ 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 { IFileSystem } from '../../common/platform/types'; +import { traceError } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; +import { shellExec } from '../../common/process/rawProcessApis'; +import { createDeferred } from '../../common/utils/async'; +import { getScriptsForShell } from './deactivateScripts'; export const terminalDeactivationPromptKey = 'TERMINAL_DEACTIVATION_PROMPT_KEY'; - @injectable() export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; + private readonly codeCLI: string; + constructor( @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, @@ -28,8 +41,12 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, @inject(IBrowserService) private readonly browserService: IBrowserService, @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, + @inject(IFileSystem) private readonly fs: IFileSystem, + @inject(IDocumentManager) private readonly documentManager: IDocumentManager, @inject(IExperimentService) private readonly experimentService: IExperimentService, - ) {} + ) { + this.codeCLI = this.appEnvironment.channel === 'insiders' ? 'code-insiders' : 'code'; + } public async activate(): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { @@ -54,19 +71,71 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv if (interpreter?.type !== PythonEnvType.Virtual) { return; } - await this.notifyUsers(); + await this.notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex)); }), ); } - private async notifyUsers(): Promise { + private async notifyUsers(shellType: TerminalShellType): Promise { const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState( - terminalDeactivationPromptKey, + `${terminalDeactivationPromptKey}-${shellType}`, true, ); if (!notificationPromptEnabled.value) { return; } + const scriptInfo = getScriptsForShell(shellType); + if (!scriptInfo) { + await this.showGeneralNotification(notificationPromptEnabled); + return; + } + const { initScriptPath, source, destination, initContents } = scriptInfo; + const name = path.basename(initScriptPath); + const prompts = [`Edit ${name}`, Common.doNotShowAgain]; + const selection = await this.appShell.showWarningMessage( + Interpreters.terminalDeactivateShellSpecificPrompt.format(name), + ...prompts, + ); + if (!selection) { + return; + } + if (selection === prompts[0]) { + await this.fs.copyFile(source, destination); + await this.openScriptWithEdits(initScriptPath, initContents); + // await notificationPromptEnabled.updateValue(false); + } + if (selection === prompts[1]) { + await notificationPromptEnabled.updateValue(false); + } + } + + private async openScriptWithEdits(scriptPath: string, content: string) { + const document = await this.openScript(scriptPath); + const editorEdit = new WorkspaceEdit(); + content = `\n +# >>> VSCode venv deactivate hook >>> +${content} +# <<< VSCode venv deactivate hook <<<`; + const editor = await window.showTextDocument(document); + editorEdit.insert(document.uri, new Position(document.lineCount, 0), content); + workspace.applyEdit(editorEdit); // Reveal the edits + const start = new Position(document.lineCount - 1, 0); + const end = new Position(document.lineCount + content.split('\n').length - 2, 0); + editor.revealRange(new Range(start, end), TextEditorRevealType.AtTop); + } + + private async openScript(scriptPath: string) { + const deferred = createDeferred(); + this.documentManager.onDidChangeActiveTextEditor((e) => { + if (e) { + deferred.resolve(e.document); + } + }); + await shellExec(`${this.codeCLI} ${scriptPath}`, { shell: this.appEnvironment.shell }); + return deferred.promise; + } + + private async showGeneralNotification(notificationPromptEnabled: IPersistentState): Promise { const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain]; const telemetrySelections: ['See Instructions', 'Done, it works', "Don't show again"] = [ 'See Instructions', diff --git a/src/client/terminals/envCollectionActivation/deactivateScripts.ts b/src/client/terminals/envCollectionActivation/deactivateScripts.ts new file mode 100644 index 000000000000..0afd0a5b25dc --- /dev/null +++ b/src/client/terminals/envCollectionActivation/deactivateScripts.ts @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { _SCRIPTS_DIR } from '../../common/process/internal/scripts/constants'; +import { TerminalShellType } from '../../common/terminal/types'; + +type ShellScripts = { + /** + * Source deactivate script to copy. + */ + source: string; + /** + * Destination to copy deactivate script to. + */ + destination: string; + /** + * Path to init script for the shell. + */ + initScriptPath: string; + /** + * Contents to add to init script. + */ + initContents: string; +}; + +// eslint-disable-next-line global-require +const untildify: (value: string) => string = require('untildify'); + +export function getScriptsForShell(shellType: TerminalShellType): ShellScripts | undefined { + switch (shellType) { + case TerminalShellType.bash: + return { + source: path.join(_SCRIPTS_DIR, 'deactivate'), + destination: untildify('~/.vscode-python/deactivate'), + initScriptPath: untildify(`~/.bashrc`), + initContents: `source ~/.vscode-python/deactivate`, + }; + case TerminalShellType.powershell: + return { + source: path.join(_SCRIPTS_DIR, 'deactivate.ps1'), + destination: untildify('~/.vscode-python/deactivate.ps1'), + initScriptPath: '$Profile', + initContents: `source ~/.vscode-python/deactivate.ps1`, + }; + case TerminalShellType.zsh: + return { + source: path.join(_SCRIPTS_DIR, 'deactivate'), + destination: untildify('~/.vscode-python/deactivate'), + initScriptPath: untildify(`~/.zshrc`), + initContents: `source ~/.vscode-python/deactivate`, + }; + case TerminalShellType.fish: + return { + source: path.join(_SCRIPTS_DIR, 'deactivate.fish'), + destination: untildify('~/.vscode-python/deactivate.fish'), + initScriptPath: '$__fish_config_dir/config.fish', + initContents: `source ~/.vscode-python/deactivate.fish`, + }; + case TerminalShellType.cshell: + return { + source: path.join(_SCRIPTS_DIR, 'deactivate.csh'), + destination: untildify('~/.vscode-python/deactivate.csh'), + initScriptPath: untildify(`~/.cshrc`), + initContents: `source ~/.vscode-python/deactivate.csh`, + }; + default: + return undefined; + } +} From ba8d8d5014afbc5527750179b3b0eadaacdd79b1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 06:16:58 +0000 Subject: [PATCH 04/38] Cleean up --- .../deactivatePrompt.ts | 18 ++- .../deactivateScripts.ts | 114 +++++++++++------- 2 files changed, 81 insertions(+), 51 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 6856af816dd3..907612784b81 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -3,7 +3,6 @@ import { inject, injectable } from 'inversify'; import { Position, TextDocument, Uri, window, workspace, WorkspaceEdit, Range, TextEditorRevealType } from 'vscode'; -import * as path from 'path'; import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types'; import { IBrowserService, @@ -25,7 +24,7 @@ import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { shellExec } from '../../common/process/rawProcessApis'; import { createDeferred } from '../../common/utils/async'; -import { getScriptsForShell } from './deactivateScripts'; +import { getDeactivateShellInfo } from './deactivateScripts'; export const terminalDeactivationPromptKey = 'TERMINAL_DEACTIVATION_PROMPT_KEY'; @injectable() @@ -84,16 +83,15 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv if (!notificationPromptEnabled.value) { return; } - const scriptInfo = getScriptsForShell(shellType); + const scriptInfo = getDeactivateShellInfo(shellType); if (!scriptInfo) { await this.showGeneralNotification(notificationPromptEnabled); return; } - const { initScriptPath, source, destination, initContents } = scriptInfo; - const name = path.basename(initScriptPath); - const prompts = [`Edit ${name}`, Common.doNotShowAgain]; + const { initScript, source, destination } = scriptInfo; + const prompts = [`Edit ${initScript.displayName}`, Common.doNotShowAgain]; const selection = await this.appShell.showWarningMessage( - Interpreters.terminalDeactivateShellSpecificPrompt.format(name), + Interpreters.terminalDeactivateShellSpecificPrompt.format(initScript.displayName), ...prompts, ); if (!selection) { @@ -101,7 +99,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv } if (selection === prompts[0]) { await this.fs.copyFile(source, destination); - await this.openScriptWithEdits(initScriptPath, initContents); + await this.openScriptWithEdits(initScript.path, initScript.contents); // await notificationPromptEnabled.updateValue(false); } if (selection === prompts[1]) { @@ -119,8 +117,8 @@ ${content} const editor = await window.showTextDocument(document); editorEdit.insert(document.uri, new Position(document.lineCount, 0), content); workspace.applyEdit(editorEdit); // Reveal the edits - const start = new Position(document.lineCount - 1, 0); - const end = new Position(document.lineCount + content.split('\n').length - 2, 0); + const start = new Position(document.lineCount - 3, 0); + const end = new Position(document.lineCount, 0); editor.revealRange(new Range(start, end), TextEditorRevealType.AtTop); } diff --git a/src/client/terminals/envCollectionActivation/deactivateScripts.ts b/src/client/terminals/envCollectionActivation/deactivateScripts.ts index 0afd0a5b25dc..7aba968cf767 100644 --- a/src/client/terminals/envCollectionActivation/deactivateScripts.ts +++ b/src/client/terminals/envCollectionActivation/deactivateScripts.ts @@ -1,70 +1,102 @@ +/* eslint-disable no-case-declarations */ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as path from 'path'; +import * as paths from 'path'; import { _SCRIPTS_DIR } from '../../common/process/internal/scripts/constants'; import { TerminalShellType } from '../../common/terminal/types'; -type ShellScripts = { +type InitScript = { /** - * Source deactivate script to copy. + * Display name of init script for the shell. */ - source: string; + displayName: string; /** - * Destination to copy deactivate script to. + * Path to init script for the shell. */ - destination: string; + path: string; +}; + +type DeactivateShellInfo = { /** - * Path to init script for the shell. + * Source deactivate script to copy. */ - initScriptPath: string; + source: string; /** - * Contents to add to init script. + * Destination to copy deactivate script to. */ - initContents: string; + destination: string; + initScript: InitScript & { + /** + * Contents to add to init script. + */ + contents: string; + }; }; // eslint-disable-next-line global-require const untildify: (value: string) => string = require('untildify'); -export function getScriptsForShell(shellType: TerminalShellType): ShellScripts | undefined { +export function getDeactivateShellInfo(shellType: TerminalShellType): DeactivateShellInfo | undefined { switch (shellType) { case TerminalShellType.bash: - return { - source: path.join(_SCRIPTS_DIR, 'deactivate'), - destination: untildify('~/.vscode-python/deactivate'), - initScriptPath: untildify(`~/.bashrc`), - initContents: `source ~/.vscode-python/deactivate`, - }; + return buildInfo( + 'deactivate', + { + displayName: '~/.bashrc', + path: untildify('~/.bashrc'), + }, + `source {0}`, + ); case TerminalShellType.powershell: - return { - source: path.join(_SCRIPTS_DIR, 'deactivate.ps1'), - destination: untildify('~/.vscode-python/deactivate.ps1'), - initScriptPath: '$Profile', - initContents: `source ~/.vscode-python/deactivate.ps1`, - }; + return buildInfo( + 'deactivate.ps1', + { + displayName: '$Profile', + path: untildify('$Profile'), + }, + `& "{0}"`, + ); case TerminalShellType.zsh: - return { - source: path.join(_SCRIPTS_DIR, 'deactivate'), - destination: untildify('~/.vscode-python/deactivate'), - initScriptPath: untildify(`~/.zshrc`), - initContents: `source ~/.vscode-python/deactivate`, - }; + return buildInfo( + 'deactivate', + { + displayName: '~/.zshrc', + path: untildify('~/.zshrc'), + }, + `source {0}`, + ); case TerminalShellType.fish: - return { - source: path.join(_SCRIPTS_DIR, 'deactivate.fish'), - destination: untildify('~/.vscode-python/deactivate.fish'), - initScriptPath: '$__fish_config_dir/config.fish', - initContents: `source ~/.vscode-python/deactivate.fish`, - }; + return buildInfo( + 'deactivate.fish', + { + displayName: 'config.fish', + path: untildify('$__fish_config_dir/config.fish'), + }, + `source {0}`, + ); case TerminalShellType.cshell: - return { - source: path.join(_SCRIPTS_DIR, 'deactivate.csh'), - destination: untildify('~/.vscode-python/deactivate.csh'), - initScriptPath: untildify(`~/.cshrc`), - initContents: `source ~/.vscode-python/deactivate.csh`, - }; + return buildInfo( + 'deactivate.csh', + { + displayName: '~/.cshrc', + path: untildify('~/.cshrc'), + }, + `source {0}`, + ); default: return undefined; } } + +function buildInfo(scriptName: string, initScript: InitScript, scriptCommandFormat: string) { + const scriptPath = `~/.vscode-python/${scriptName}`; + return { + source: paths.join(_SCRIPTS_DIR, scriptName), + destination: untildify(scriptPath), + initScript: { + ...initScript, + contents: scriptCommandFormat.format(scriptPath), + }, + }; +} From eb0b909c6618eaf9f8921c2bddfc636cf73192c9 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 06:56:28 +0000 Subject: [PATCH 05/38] More changes --- .../deactivatePrompt.ts | 2 +- .../deactivateScripts.ts | 6 ++--- .../envCollectionActivation/service.ts | 26 ++++++++++++++----- .../shellIntegration.ts | 13 ++++++++++ 4 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 src/client/terminals/envCollectionActivation/shellIntegration.ts diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 907612784b81..05f45e99e446 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -100,7 +100,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv if (selection === prompts[0]) { await this.fs.copyFile(source, destination); await this.openScriptWithEdits(initScript.path, initScript.contents); - // await notificationPromptEnabled.updateValue(false); + await notificationPromptEnabled.updateValue(false); } if (selection === prompts[1]) { await notificationPromptEnabled.updateValue(false); diff --git a/src/client/terminals/envCollectionActivation/deactivateScripts.ts b/src/client/terminals/envCollectionActivation/deactivateScripts.ts index 7aba968cf767..e73c03c35b6d 100644 --- a/src/client/terminals/envCollectionActivation/deactivateScripts.ts +++ b/src/client/terminals/envCollectionActivation/deactivateScripts.ts @@ -89,10 +89,10 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate } } -function buildInfo(scriptName: string, initScript: InitScript, scriptCommandFormat: string) { - const scriptPath = `~/.vscode-python/${scriptName}`; +function buildInfo(deactivate: string, initScript: InitScript, scriptCommandFormat: string) { + const scriptPath = `~/.vscode-python/${deactivate}`; return { - source: paths.join(_SCRIPTS_DIR, scriptName), + source: paths.join(_SCRIPTS_DIR, deactivate), destination: untildify(scriptPath), initScript: { ...initScript, diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 0038511a3dbb..0ad935cf25a4 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -40,6 +40,7 @@ import { OSType } from '../../common/utils/platform'; import { normCase } from '../../common/platform/fs-paths'; import { PythonEnvType } from '../../pythonEnvironments/base/info'; import { ITerminalEnvVarCollectionService } from '../types'; +import { ShellIntegrationShells } from './shellIntegration'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService, ITerminalEnvVarCollectionService { @@ -266,9 +267,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 should be set but no PS1 was set. return; } - const config = this.workspaceService - .getConfiguration('terminal') - .get('integrated.shellIntegration.enabled'); + const config = this.isShellIntegrationEnabled(); if (!config) { traceVerbose('PS1 is not set when shell integration is disabled.'); return; @@ -324,11 +323,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } private getPrependOptions(): EnvironmentVariableMutatorOptions { - const value = this.workspaceService - .getConfiguration('terminal') - .get('integrated.shellIntegration.enabled'); + const enabled = this.isShellIntegrationEnabled(); // Make sure to prepend either at shell integration or process creation, not both. - return value + return enabled ? { applyAtShellIntegration: true, applyAtProcessCreation: false, @@ -339,6 +336,21 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ }; } + private isShellIntegrationEnabled(): boolean { + const isEnabled = this.workspaceService + .getConfiguration('terminal') + .get('integrated.shellIntegration.enabled')!; + if ( + isEnabled && + ShellIntegrationShells.includes(identifyShellFromShellPath(this.applicationEnvironment.shell)) + ) { + // Unfortunately shell integration could still've failed in remote scenarios, we can't know for sure: + // https://code.visualstudio.com/docs/terminal/shell-integration#_automatic-script-injection + return true; + } + return false; + } + private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) { const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection; return envVarCollection.getScoped(scope); diff --git a/src/client/terminals/envCollectionActivation/shellIntegration.ts b/src/client/terminals/envCollectionActivation/shellIntegration.ts new file mode 100644 index 000000000000..1be2501595a4 --- /dev/null +++ b/src/client/terminals/envCollectionActivation/shellIntegration.ts @@ -0,0 +1,13 @@ +import { TerminalShellType } from '../../common/terminal/types'; + +/** + * This is a list of shells which support shell integration: + * https://code.visualstudio.com/docs/terminal/shell-integration + */ +export const ShellIntegrationShells = [ + TerminalShellType.powershell, + TerminalShellType.powershellCore, + TerminalShellType.bash, + TerminalShellType.zsh, + TerminalShellType.fish, +]; From 11f789ad45208279bb0ceacf048c73433f777b0f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 07:19:59 +0000 Subject: [PATCH 06/38] Yes --- .../terminals/envCollectionActivation/service.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 0ad935cf25a4..8c381126759f 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -267,7 +267,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 should be set but no PS1 was set. return; } - const config = this.isShellIntegrationEnabled(); + const config = this.isShellIntegrationActive(); if (!config) { traceVerbose('PS1 is not set when shell integration is disabled.'); return; @@ -323,9 +323,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } private getPrependOptions(): EnvironmentVariableMutatorOptions { - const enabled = this.isShellIntegrationEnabled(); - // Make sure to prepend either at shell integration or process creation, not both. - return enabled + const isActive = this.isShellIntegrationActive(); + // Ideally we would want to prepend exactly once, either at shell integration or process creation. + // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available. + return isActive ? { applyAtShellIntegration: true, applyAtProcessCreation: false, @@ -336,7 +337,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ }; } - private isShellIntegrationEnabled(): boolean { + private isShellIntegrationActive(): boolean { const isEnabled = this.workspaceService .getConfiguration('terminal') .get('integrated.shellIntegration.enabled')!; From bca27e1d5ab19365d20c145170396867a066f4b8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 12:00:00 -0700 Subject: [PATCH 07/38] Changes based on powershell --- .../deactivatePrompt.ts | 7 +++--- .../deactivateScripts.ts | 22 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 05f45e99e446..0383f1a4c67a 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -110,16 +110,15 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv private async openScriptWithEdits(scriptPath: string, content: string) { const document = await this.openScript(scriptPath); const editorEdit = new WorkspaceEdit(); - content = `\n + content = ` # >>> VSCode venv deactivate hook >>> ${content} # <<< VSCode venv deactivate hook <<<`; const editor = await window.showTextDocument(document); editorEdit.insert(document.uri, new Position(document.lineCount, 0), content); workspace.applyEdit(editorEdit); // Reveal the edits - const start = new Position(document.lineCount - 3, 0); - const end = new Position(document.lineCount, 0); - editor.revealRange(new Range(start, end), TextEditorRevealType.AtTop); + const lastLine = new Position(document.lineCount, 0); + editor.revealRange(new Range(lastLine, lastLine), TextEditorRevealType.AtTop); } private async openScript(scriptPath: string) { diff --git a/src/client/terminals/envCollectionActivation/deactivateScripts.ts b/src/client/terminals/envCollectionActivation/deactivateScripts.ts index e73c03c35b6d..c281d635d8b6 100644 --- a/src/client/terminals/envCollectionActivation/deactivateScripts.ts +++ b/src/client/terminals/envCollectionActivation/deactivateScripts.ts @@ -2,7 +2,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as paths from 'path'; +import * as path from 'path'; import { _SCRIPTS_DIR } from '../../common/process/internal/scripts/constants'; import { TerminalShellType } from '../../common/terminal/types'; @@ -12,18 +12,18 @@ type InitScript = { */ displayName: string; /** - * Path to init script for the shell. + * Shell specific variable or path which is equivalent to the full path to init script. */ path: string; }; type DeactivateShellInfo = { /** - * Source deactivate script to copy. + * Full path to source deactivate script to copy. */ source: string; /** - * Destination to copy deactivate script to. + * Full path to destination to copy deactivate script to. */ destination: string; initScript: InitScript & { @@ -44,7 +44,7 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate 'deactivate', { displayName: '~/.bashrc', - path: untildify('~/.bashrc'), + path: '~/.bashrc', }, `source {0}`, ); @@ -52,8 +52,8 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate return buildInfo( 'deactivate.ps1', { - displayName: '$Profile', - path: untildify('$Profile'), + displayName: 'Powershell Profile', + path: '$Profile', }, `& "{0}"`, ); @@ -62,7 +62,7 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate 'deactivate', { displayName: '~/.zshrc', - path: untildify('~/.zshrc'), + path: '~/.zshrc', }, `source {0}`, ); @@ -71,7 +71,7 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate 'deactivate.fish', { displayName: 'config.fish', - path: untildify('$__fish_config_dir/config.fish'), + path: '$__fish_config_dir/config.fish', }, `source {0}`, ); @@ -80,7 +80,7 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate 'deactivate.csh', { displayName: '~/.cshrc', - path: untildify('~/.cshrc'), + path: '~/.cshrc', }, `source {0}`, ); @@ -92,7 +92,7 @@ export function getDeactivateShellInfo(shellType: TerminalShellType): Deactivate function buildInfo(deactivate: string, initScript: InitScript, scriptCommandFormat: string) { const scriptPath = `~/.vscode-python/${deactivate}`; return { - source: paths.join(_SCRIPTS_DIR, deactivate), + source: path.join(_SCRIPTS_DIR, deactivate), destination: untildify(scriptPath), initScript: { ...initScript, From 91d634c12df8f2edec478122df3866311a586491 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 19:06:58 +0000 Subject: [PATCH 08/38] Log when not in experiment due to multiroot+scenario scenario --- src/client/common/experiments/helpers.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts index bae96b222eb6..3f2b55e18657 100644 --- a/src/client/common/experiments/helpers.ts +++ b/src/client/common/experiments/helpers.ts @@ -7,10 +7,12 @@ import { env, workspace } from 'vscode'; import { IExperimentService } from '../types'; import { TerminalEnvVarActivation } from './groups'; import { isTestExecution } from '../constants'; +import { traceInfo } from '../../logging'; 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. + traceInfo('Not enabling terminal env var experiment in remote workspaces'); return false; } if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) { From 1c6a8bc4ca7ab6c78f36a8b6082c2d2cc4cf8e07 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 19:28:01 +0000 Subject: [PATCH 09/38] Log process variables at the start --- src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts b/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts index 468c2dc72a01..b4dcfe36e095 100644 --- a/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts +++ b/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts @@ -91,7 +91,10 @@ export class ActivatedEnvironmentLaunch implements IActivatedEnvironmentLaunch { } if (process.env.VSCODE_CLI !== '1') { // We only want to select the interpreter if VS Code was launched from the command line. - traceVerbose('VS Code was not launched from the command line, not selecting activated interpreter'); + traceVerbose( + 'VS Code was not launched from the command line, not selecting activated interpreter', + JSON.stringify(process.env, undefined, 4), + ); return undefined; } const prefix = await this.getPrefixOfSelectedActivatedEnv(); From 648c6acf5455ad4d713d3b9f579f843b3e0710d4 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 19:35:48 +0000 Subject: [PATCH 10/38] Use worksapceFolder to judge mutlirootworkspace --- src/client/common/experiments/helpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts index 3f2b55e18657..126b7218cf18 100644 --- a/src/client/common/experiments/helpers.ts +++ b/src/client/common/experiments/helpers.ts @@ -10,9 +10,9 @@ import { isTestExecution } from '../constants'; import { traceInfo } from '../../logging'; export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean { - if (!isTestExecution() && workspace.workspaceFile && env.remoteName) { + if (!isTestExecution() && workspace.workspaceFolders && env.remoteName && workspace.workspaceFolders?.length > 1) { // TODO: Remove this if statement once https://github.com/microsoft/vscode/issues/180486 is fixed. - traceInfo('Not enabling terminal env var experiment in remote workspaces'); + traceInfo('Not enabling terminal env var experiment in multiroot remote workspaces'); return false; } if (!experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)) { From 8aaf46c0b2f16f7fd06844f60395a545ff93450c Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 21:36:21 +0000 Subject: [PATCH 11/38] Fine tuen --- src/client/common/experiments/helpers.ts | 2 +- src/client/common/utils/localize.ts | 2 +- .../envCollectionActivation/deactivatePrompt.ts | 17 +++++++++++++---- .../envCollectionActivation/indicatorPrompt.ts | 6 ++++++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/client/common/experiments/helpers.ts b/src/client/common/experiments/helpers.ts index 126b7218cf18..f6ae39d260f5 100644 --- a/src/client/common/experiments/helpers.ts +++ b/src/client/common/experiments/helpers.ts @@ -10,7 +10,7 @@ import { isTestExecution } from '../constants'; import { traceInfo } from '../../logging'; export function inTerminalEnvVarExperiment(experimentService: IExperimentService): boolean { - if (!isTestExecution() && workspace.workspaceFolders && env.remoteName && workspace.workspaceFolders?.length > 1) { + if (!isTestExecution() && env.remoteName && workspace.workspaceFolders && workspace.workspaceFolders.length > 1) { // TODO: Remove this if statement once https://github.com/microsoft/vscode/issues/180486 is fixed. traceInfo('Not enabling terminal env var experiment in multiroot remote workspaces'); return false; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 1fc6186a9fa2..e8fd41bd3bee 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -202,7 +202,7 @@ export namespace Interpreters { '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 terminalDeactivateShellSpecificPrompt = l10n.t( - 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved by appending a line to "{0}" and restarting the shell. [Learn more](https://aka.ms/AAmx2ft).', + 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved by appending a line to "{0}". Be sure to restart the shell afterward. [Learn more](https://aka.ms/AAmx2ft).', ); export const deactivateDoneButton = l10n.t('Done, it works'); export const activatedCondaEnvLaunch = l10n.t( diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 0383f1a4c67a..9ebce2fbeae9 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -23,8 +23,9 @@ import { traceError } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { shellExec } from '../../common/process/rawProcessApis'; -import { createDeferred } from '../../common/utils/async'; +import { createDeferred, sleep } from '../../common/utils/async'; import { getDeactivateShellInfo } from './deactivateScripts'; +import { isTestExecution } from '../../common/constants'; export const terminalDeactivationPromptKey = 'TERMINAL_DEACTIVATION_PROMPT_KEY'; @injectable() @@ -51,6 +52,10 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv if (!inTerminalEnvVarExperiment(this.experimentService)) { return; } + if (!isTestExecution()) { + // Avoid showing prompt until startup completes. + await sleep(5000); + } this.disposableRegistry.push( this.appShell.onDidWriteTerminalData(async (e) => { if (!e.data.includes('deactivate')) { @@ -109,14 +114,18 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv private async openScriptWithEdits(scriptPath: string, content: string) { const document = await this.openScript(scriptPath); - const editorEdit = new WorkspaceEdit(); content = ` # >>> VSCode venv deactivate hook >>> ${content} # <<< VSCode venv deactivate hook <<<`; - const editor = await window.showTextDocument(document); + // If script already has the hook, don't add it again. + if (document.getText().includes('VSCode venv deactivate hook')) { + return; + } + const editor = await this.documentManager.showTextDocument(document); + const editorEdit = new WorkspaceEdit(); editorEdit.insert(document.uri, new Position(document.lineCount, 0), content); - workspace.applyEdit(editorEdit); // Reveal the edits + await this.documentManager.applyEdit(editorEdit); // Reveal the edits const lastLine = new Position(document.lineCount, 0); editor.revealRange(new Range(lastLine, lastLine), TextEditorRevealType.AtTop); } diff --git a/src/client/terminals/envCollectionActivation/indicatorPrompt.ts b/src/client/terminals/envCollectionActivation/indicatorPrompt.ts index bf648eefe8e9..bc4c3cc90fc0 100644 --- a/src/client/terminals/envCollectionActivation/indicatorPrompt.ts +++ b/src/client/terminals/envCollectionActivation/indicatorPrompt.ts @@ -18,6 +18,8 @@ import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; import { IInterpreterService } from '../../interpreter/contracts'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { ITerminalEnvVarCollectionService } from '../types'; +import { sleep } from '../../common/utils/async'; +import { isTestExecution } from '../../common/constants'; export const terminalEnvCollectionPromptKey = 'TERMINAL_ENV_COLLECTION_PROMPT_KEY'; @@ -42,6 +44,10 @@ export class TerminalIndicatorPrompt implements IExtensionSingleActivationServic if (!inTerminalEnvVarExperiment(this.experimentService)) { return; } + if (!isTestExecution()) { + // Avoid showing prompt until startup completes. + await sleep(5000); + } this.disposableRegistry.push( this.terminalManager.onDidOpenTerminal(async (terminal) => { const cwd = From dff84dce8bf718120f90a616061a7b2b41e52f6e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 21:55:49 +0000 Subject: [PATCH 12/38] Add progress notification --- .../common/application/progressService.ts | 36 ++++++++++++++++ src/client/common/utils/localize.ts | 1 + .../deactivatePrompt.ts | 9 +++- .../envCollectionActivation/service.ts | 41 ++++--------------- 4 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 src/client/common/application/progressService.ts diff --git a/src/client/common/application/progressService.ts b/src/client/common/application/progressService.ts new file mode 100644 index 000000000000..dcb314584822 --- /dev/null +++ b/src/client/common/application/progressService.ts @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ProgressLocation, ProgressOptions } from 'vscode'; +import { Deferred, createDeferred } from '../utils/async'; +import { IApplicationShell } from './types'; + +export class ProgressService { + private deferred: Deferred | undefined; + + constructor(private readonly shell: IApplicationShell, private readonly title: string) {} + + public showProgress(): void { + if (!this.deferred) { + this.createProgress(); + } + } + + public hideProgress(): void { + if (this.deferred) { + this.deferred.resolve(); + this.deferred = undefined; + } + } + + private createProgress() { + const progressOptions: ProgressOptions = { + location: ProgressLocation.Window, + title: this.title, + }; + this.shell.withProgress(progressOptions, () => { + this.deferred = createDeferred(); + return this.deferred.promise; + }); + } +} diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index e8fd41bd3bee..9aef895a405a 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -198,6 +198,7 @@ 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 terminalDeactivateProgress = l10n.t('Modifying script...'); 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.', ); diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 9ebce2fbeae9..f81b9133c534 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { Position, TextDocument, Uri, window, workspace, WorkspaceEdit, Range, TextEditorRevealType } from 'vscode'; +import { Position, TextDocument, Uri, WorkspaceEdit, Range, TextEditorRevealType } from 'vscode'; import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types'; import { IBrowserService, @@ -26,6 +26,7 @@ import { shellExec } from '../../common/process/rawProcessApis'; import { createDeferred, sleep } from '../../common/utils/async'; import { getDeactivateShellInfo } from './deactivateScripts'; import { isTestExecution } from '../../common/constants'; +import { ProgressService } from '../../common/application/progressService'; export const terminalDeactivationPromptKey = 'TERMINAL_DEACTIVATION_PROMPT_KEY'; @injectable() @@ -34,6 +35,8 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv private readonly codeCLI: string; + private readonly progressService: ProgressService; + constructor( @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory, @@ -43,9 +46,11 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, @inject(IFileSystem) private readonly fs: IFileSystem, @inject(IDocumentManager) private readonly documentManager: IDocumentManager, + @inject(IApplicationShell) private readonly shell: IApplicationShell, @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { this.codeCLI = this.appEnvironment.channel === 'insiders' ? 'code-insiders' : 'code'; + this.progressService = new ProgressService(this.shell, Interpreters.terminalDeactivateProgress); } public async activate(): Promise { @@ -76,6 +81,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv return; } await this.notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex)); + this.progressService.hideProgress(); // Hide the progress bar if it exists. }), ); } @@ -103,6 +109,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv return; } if (selection === prompts[0]) { + this.progressService.showProgress(); await this.fs.copyFile(source, destination); await this.openScriptWithEdits(initScript.path, initScript.contents); await notificationPromptEnabled.updateValue(false); diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 8c381126759f..dfaa74cd9c71 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -4,8 +4,6 @@ import * as path from 'path'; import { inject, injectable } from 'inversify'; import { - ProgressOptions, - ProgressLocation, MarkdownString, WorkspaceFolder, GlobalEnvironmentVariableCollection, @@ -26,9 +24,8 @@ import { IConfigurationService, IPathUtils, } from '../../common/types'; -import { Deferred, createDeferred } from '../../common/utils/async'; import { Interpreters } from '../../common/utils/localize'; -import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging'; +import { traceError, traceVerbose, traceWarn } from '../../logging'; import { IInterpreterService } from '../../interpreter/contracts'; import { defaultShells } from '../../interpreter/activation/service'; import { IEnvironmentActivationService } from '../../interpreter/activation/types'; @@ -41,6 +38,7 @@ import { normCase } from '../../common/platform/fs-paths'; import { PythonEnvType } from '../../pythonEnvironments/base/info'; import { ITerminalEnvVarCollectionService } from '../types'; import { ShellIntegrationShells } from './shellIntegration'; +import { ProgressService } from '../../common/application/progressService'; @injectable() export class TerminalEnvVarCollectionService implements IExtensionActivationService, ITerminalEnvVarCollectionService { @@ -58,8 +56,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ TerminalShellType.fish, ]; - private deferred: Deferred | undefined; - private registeredOnce = false; /** @@ -67,6 +63,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ */ private processEnvVars: EnvironmentVariables | undefined; + private readonly progressService: ProgressService; + private separator: string; constructor( @@ -83,6 +81,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IPathUtils) private readonly pathUtils: IPathUtils, ) { this.separator = platform.osType === OSType.Windows ? ';' : ':'; + this.progressService = new ProgressService(this.shell, Interpreters.activatingTerminals); } public async activate(resource: Resource): Promise { @@ -129,9 +128,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } public async _applyCollection(resource: Resource, shell?: string): Promise { - this.showProgress(); + this.progressService.showProgress(); await this._applyCollectionImpl(resource, shell); - this.hideProgress(); + this.progressService.hideProgress(); } private async _applyCollectionImpl(resource: Resource, shell = this.applicationEnvironment.shell): Promise { @@ -368,32 +367,6 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } return workspaceFolder; } - - @traceDecoratorVerbose('Display activating terminals') - private showProgress(): void { - if (!this.deferred) { - this.createProgress(); - } - } - - @traceDecoratorVerbose('Hide activating terminals') - private hideProgress(): void { - if (this.deferred) { - this.deferred.resolve(); - this.deferred = undefined; - } - } - - private createProgress() { - const progressOptions: ProgressOptions = { - location: ProgressLocation.Window, - title: Interpreters.activatingTerminals, - }; - this.shell.withProgress(progressOptions, () => { - this.deferred = createDeferred(); - return this.deferred.promise; - }); - } } function shouldPS1BeSet(type: PythonEnvType | undefined, env: EnvironmentVariables): boolean { From 881c8d41566346addb1b24dc2528d5c547b7e8c1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 15:19:31 -0700 Subject: [PATCH 13/38] Make progress nicer --- src/client/common/application/progressService.ts | 16 ++++++---------- src/client/common/utils/localize.ts | 2 +- .../envCollectionActivation/deactivatePrompt.ts | 9 ++++++--- .../terminals/envCollectionActivation/service.ts | 8 ++++++-- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/client/common/application/progressService.ts b/src/client/common/application/progressService.ts index dcb314584822..fb19cad1136c 100644 --- a/src/client/common/application/progressService.ts +++ b/src/client/common/application/progressService.ts @@ -1,18 +1,18 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { ProgressLocation, ProgressOptions } from 'vscode'; +import { ProgressOptions } from 'vscode'; import { Deferred, createDeferred } from '../utils/async'; import { IApplicationShell } from './types'; export class ProgressService { private deferred: Deferred | undefined; - constructor(private readonly shell: IApplicationShell, private readonly title: string) {} + constructor(private readonly shell: IApplicationShell) {} - public showProgress(): void { + public showProgress(options: ProgressOptions): void { if (!this.deferred) { - this.createProgress(); + this.createProgress(options); } } @@ -23,12 +23,8 @@ export class ProgressService { } } - private createProgress() { - const progressOptions: ProgressOptions = { - location: ProgressLocation.Window, - title: this.title, - }; - this.shell.withProgress(progressOptions, () => { + private createProgress(options: ProgressOptions) { + this.shell.withProgress(options, () => { this.deferred = createDeferred(); return this.deferred.promise; }); diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 9aef895a405a..3c3d288e30b8 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -198,7 +198,7 @@ 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 terminalDeactivateProgress = l10n.t('Modifying script...'); + export const terminalDeactivateProgress = l10n.t('Editing {0}...'); 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.', ); diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index f81b9133c534..6f10f79d7f55 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { Position, TextDocument, Uri, WorkspaceEdit, Range, TextEditorRevealType } from 'vscode'; +import { Position, TextDocument, Uri, WorkspaceEdit, Range, TextEditorRevealType, ProgressLocation } from 'vscode'; import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types'; import { IBrowserService, @@ -50,7 +50,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { this.codeCLI = this.appEnvironment.channel === 'insiders' ? 'code-insiders' : 'code'; - this.progressService = new ProgressService(this.shell, Interpreters.terminalDeactivateProgress); + this.progressService = new ProgressService(this.shell); } public async activate(): Promise { @@ -109,7 +109,10 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv return; } if (selection === prompts[0]) { - this.progressService.showProgress(); + this.progressService.showProgress({ + location: ProgressLocation.Window, + title: Interpreters.terminalDeactivateProgress.format(initScript.displayName), + }); await this.fs.copyFile(source, destination); await this.openScriptWithEdits(initScript.path, initScript.contents); await notificationPromptEnabled.updateValue(false); diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index dfaa74cd9c71..23f7e6c9f745 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -9,6 +9,7 @@ import { GlobalEnvironmentVariableCollection, EnvironmentVariableScope, EnvironmentVariableMutatorOptions, + ProgressLocation, } from 'vscode'; import { pathExists } from 'fs-extra'; import { IExtensionActivationService } from '../../activation/types'; @@ -81,7 +82,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IPathUtils) private readonly pathUtils: IPathUtils, ) { this.separator = platform.osType === OSType.Windows ? ';' : ':'; - this.progressService = new ProgressService(this.shell, Interpreters.activatingTerminals); + this.progressService = new ProgressService(this.shell); } public async activate(resource: Resource): Promise { @@ -128,7 +129,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } public async _applyCollection(resource: Resource, shell?: string): Promise { - this.progressService.showProgress(); + this.progressService.showProgress({ + location: ProgressLocation.Window, + title: Interpreters.terminalDeactivateProgress.format(Interpreters.activatingTerminals), + }); await this._applyCollectionImpl(resource, shell); this.progressService.hideProgress(); } From 18ac7ee3ec8bc8b0817baf9152efdde7d4ced91b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 22:32:07 +0000 Subject: [PATCH 14/38] Nit --- .../terminals/envCollectionActivation/deactivatePrompt.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 6f10f79d7f55..df85985c3372 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -124,12 +124,13 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv private async openScriptWithEdits(scriptPath: string, content: string) { const document = await this.openScript(scriptPath); + const hookMarker = 'VSCode venv deactivate hook'; content = ` -# >>> VSCode venv deactivate hook >>> +# >>> ${hookMarker} >>> ${content} -# <<< VSCode venv deactivate hook <<<`; +# <<< ${hookMarker} <<<`; // If script already has the hook, don't add it again. - if (document.getText().includes('VSCode venv deactivate hook')) { + if (document.getText().includes(hookMarker)) { return; } const editor = await this.documentManager.showTextDocument(document); From 4959e4d069015a321d649ace4309b6689151f1bd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 23:44:49 +0000 Subject: [PATCH 15/38] Add tests for progress service --- .../application/progressService.unit.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 src/test/common/application/progressService.unit.test.ts diff --git a/src/test/common/application/progressService.unit.test.ts b/src/test/common/application/progressService.unit.test.ts new file mode 100644 index 000000000000..b9c49ccb4060 --- /dev/null +++ b/src/test/common/application/progressService.unit.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { assert, expect } from 'chai'; +import { anything, capture, instance, mock, when } from 'ts-mockito'; +import { CancellationToken, Progress, ProgressLocation, ProgressOptions } from 'vscode'; +import { ApplicationShell } from '../../../client/common/application/applicationShell'; +import { ProgressService } from '../../../client/common/application/progressService'; +import { IApplicationShell } from '../../../client/common/application/types'; +import { createDeferred, createDeferredFromPromise, Deferred, sleep } from '../../../client/common/utils/async'; + +type ProgressTask = ( + progress: Progress<{ message?: string; increment?: number }>, + token: CancellationToken, +) => Thenable; + +suite('Progress Service', () => { + let refreshDeferred: Deferred; + let shell: ApplicationShell; + let progressService: ProgressService; + setup(() => { + refreshDeferred = createDeferred(); + shell = mock(); + progressService = new ProgressService(instance(shell)); + }); + teardown(() => { + refreshDeferred.resolve(); + }); + test('Display discovering message when refreshing interpreters for the first time', async () => { + when(shell.withProgress(anything(), anything())).thenResolve(); + const expectedOptions = { title: 'message', location: ProgressLocation.Window }; + + progressService.showProgress(expectedOptions); + + const options = capture(shell.withProgress as never).last()[0] as ProgressOptions; + assert.deepEqual(options, expectedOptions); + }); + + test('Progress message is hidden when loading has completed', async () => { + when(shell.withProgress(anything(), anything())).thenResolve(); + const options = { title: 'message', location: ProgressLocation.Window }; + progressService.showProgress(options); + + const callback = capture(shell.withProgress as never).last()[1] as ProgressTask; + const promise = callback(undefined as never, undefined as never); + const deferred = createDeferredFromPromise(promise as Promise); + await sleep(1); + expect(deferred.completed).to.be.equal(false, 'Progress disappeared before hiding it'); + progressService.hideProgress(); + await sleep(1); + expect(deferred.completed).to.be.equal(true, 'Progress did not disappear'); + }); +}); From 72a80a7678ad47ba3f896ffd8edf542cdf5ea8b7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 15 Oct 2023 00:02:45 +0000 Subject: [PATCH 16/38] Fix activating using command when deactivate is in place --- pythonFiles/deactivate.fish | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pythonFiles/deactivate.fish b/pythonFiles/deactivate.fish index 6e8dc26db81e..c652a8c1e3d7 100644 --- a/pythonFiles/deactivate.fish +++ b/pythonFiles/deactivate.fish @@ -10,12 +10,12 @@ function deactivate -d "Exit virtual environment and return to normal shell env set -e _OLD_VIRTUAL_PYTHONHOME end - if test -n "$_OLD_FISH_PROMPT_OVERRIDE" - set -e _OLD_FISH_PROMPT_OVERRIDE - if functions -q _old_fish_prompt + if test -n "$vscode_python_old_fish_prompt_OVERRIDE" + set -e vscode_python_old_fish_prompt_OVERRIDE + if functions -q vscode_python_old_fish_prompt functions -e fish_prompt - functions -c _old_fish_prompt fish_prompt - functions -e _old_fish_prompt + functions -c vscode_python_old_fish_prompt fish_prompt + functions -e vscode_python_old_fish_prompt end end @@ -29,7 +29,7 @@ end # Initialize the variables required by deactivate function set -gx _OLD_VIRTUAL_PATH $PATH if test -z "$VIRTUAL_ENV_DISABLE_PROMPT" - functions -c fish_prompt _old_fish_prompt + functions -c fish_prompt vscode_python_old_fish_prompt end if set -q PYTHONHOME set -gx _OLD_VIRTUAL_PYTHONHOME $PYTHONHOME From 5e05dc0bb2419a1559510ca95caa53fca30b5e70 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 15 Oct 2023 00:11:01 +0000 Subject: [PATCH 17/38] Don't hide progress bar in case of errror --- .../terminals/envCollectionActivation/deactivatePrompt.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index df85985c3372..21432b496ad3 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -81,7 +81,6 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv return; } await this.notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex)); - this.progressService.hideProgress(); // Hide the progress bar if it exists. }), ); } @@ -116,6 +115,7 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv await this.fs.copyFile(source, destination); await this.openScriptWithEdits(initScript.path, initScript.contents); await notificationPromptEnabled.updateValue(false); + this.progressService.hideProgress(); } if (selection === prompts[1]) { await notificationPromptEnabled.updateValue(false); From cb2b3140c07f540870f171c3e9eb0a06443da776 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sat, 14 Oct 2023 17:27:48 -0700 Subject: [PATCH 18/38] Hey --- src/client/terminals/envCollectionActivation/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 23f7e6c9f745..0c672a3d3d24 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -131,7 +131,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell?: string): Promise { this.progressService.showProgress({ location: ProgressLocation.Window, - title: Interpreters.terminalDeactivateProgress.format(Interpreters.activatingTerminals), + title: Interpreters.activatingTerminals, }); await this._applyCollectionImpl(resource, shell); this.progressService.hideProgress(); From 7649be7901d051cf84ae6db630c16bbe0fbcf491 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 18 Oct 2023 21:39:04 +0000 Subject: [PATCH 19/38] Add unit tests --- src/client/common/utils/localize.ts | 5 +- src/client/telemetry/index.ts | 18 ---- .../deactivatePrompt.ts | 47 ++------ .../deactivatePrompt.unit.test.ts | 102 +++++++++--------- 4 files changed, 56 insertions(+), 116 deletions(-) diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 3c3d288e30b8..4fe4a9c10956 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -61,6 +61,7 @@ export namespace Common { export const noIWillDoItLater = l10n.t('No, I will do it later'); export const notNow = l10n.t('Not now'); export const doNotShowAgain = l10n.t("Don't show again"); + export const editSomething = l10n.t('Edit {0}'); export const reload = l10n.t('Reload'); export const moreInfo = l10n.t('More Info'); export const learnMore = l10n.t('Learn more'); @@ -200,12 +201,8 @@ export namespace Interpreters { ); export const terminalDeactivateProgress = l10n.t('Editing {0}...'); 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 terminalDeactivateShellSpecificPrompt = l10n.t( 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved by appending a line to "{0}". Be sure to restart the shell afterward. [Learn more](https://aka.ms/AAmx2ft).', ); - 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?', ); diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 4c612b5b86bc..f69da6046254 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1291,24 +1291,6 @@ 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. */ diff --git a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts index 21432b496ad3..a455f3d12a52 100644 --- a/src/client/terminals/envCollectionActivation/deactivatePrompt.ts +++ b/src/client/terminals/envCollectionActivation/deactivatePrompt.ts @@ -4,13 +4,7 @@ import { inject, injectable } from 'inversify'; import { Position, TextDocument, Uri, WorkspaceEdit, Range, TextEditorRevealType, ProgressLocation } from 'vscode'; import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types'; -import { - IBrowserService, - IDisposableRegistry, - IExperimentService, - IPersistentState, - IPersistentStateFactory, -} from '../../common/types'; +import { IDisposableRegistry, IExperimentService, IPersistentStateFactory } from '../../common/types'; import { Common, Interpreters } from '../../common/utils/localize'; import { IExtensionSingleActivationService } from '../../activation/types'; import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers'; @@ -20,8 +14,6 @@ import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors import { TerminalShellType } from '../../common/terminal/types'; import { IFileSystem } from '../../common/platform/types'; import { traceError } from '../../logging'; -import { sendTelemetryEvent } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; import { shellExec } from '../../common/process/rawProcessApis'; import { createDeferred, sleep } from '../../common/utils/async'; import { getDeactivateShellInfo } from './deactivateScripts'; @@ -42,15 +34,13 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv @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(IFileSystem) private readonly fs: IFileSystem, @inject(IDocumentManager) private readonly documentManager: IDocumentManager, - @inject(IApplicationShell) private readonly shell: IApplicationShell, @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { this.codeCLI = this.appEnvironment.channel === 'insiders' ? 'code-insiders' : 'code'; - this.progressService = new ProgressService(this.shell); + this.progressService = new ProgressService(this.appShell); } public async activate(): Promise { @@ -80,12 +70,12 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv if (interpreter?.type !== PythonEnvType.Virtual) { return; } - await this.notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex)); + await this._notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex)); }), ); } - private async notifyUsers(shellType: TerminalShellType): Promise { + public async _notifyUsers(shellType: TerminalShellType): Promise { const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState( `${terminalDeactivationPromptKey}-${shellType}`, true, @@ -95,13 +85,13 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv } const scriptInfo = getDeactivateShellInfo(shellType); if (!scriptInfo) { - await this.showGeneralNotification(notificationPromptEnabled); + // Shell integration is not supported for these shells, in which case this workaround won't work. return; } const { initScript, source, destination } = scriptInfo; - const prompts = [`Edit ${initScript.displayName}`, Common.doNotShowAgain]; + const prompts = [Common.editSomething.format(initScript.displayName), Common.doNotShowAgain]; const selection = await this.appShell.showWarningMessage( - Interpreters.terminalDeactivateShellSpecificPrompt.format(initScript.displayName), + Interpreters.terminalDeactivatePrompt.format(initScript.displayName), ...prompts, ); if (!selection) { @@ -151,27 +141,4 @@ ${content} await shellExec(`${this.codeCLI} ${scriptPath}`, { shell: this.appEnvironment.shell }); return deferred.promise; } - - private async showGeneralNotification(notificationPromptEnabled: IPersistentState): Promise { - 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); - } - } } diff --git a/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts b/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts index acd8ee99e5d7..876a662073d6 100644 --- a/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts +++ b/src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts @@ -4,14 +4,10 @@ 'use strict'; import { mock, when, anything, instance, verify, reset } from 'ts-mockito'; -import { EventEmitter, Terminal, TerminalDataWriteEvent, Uri } from 'vscode'; -import { IApplicationEnvironment, IApplicationShell } from '../../../client/common/application/types'; -import { - IBrowserService, - IExperimentService, - IPersistentState, - IPersistentStateFactory, -} from '../../../client/common/types'; +import { EventEmitter, Terminal, TerminalDataWriteEvent, TextDocument, TextEditor, Uri } from 'vscode'; +import * as sinon from 'sinon'; +import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../../client/common/application/types'; +import { IExperimentService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types'; import { Common, Interpreters } from '../../../client/common/utils/localize'; import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups'; import { sleep } from '../../core'; @@ -20,6 +16,8 @@ import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; import { TerminalDeactivateLimitationPrompt } from '../../../client/terminals/envCollectionActivation/deactivatePrompt'; import { PythonEnvType } from '../../../client/pythonEnvironments/base/info'; import { TerminalShellType } from '../../../client/common/terminal/types'; +import { IFileSystem } from '../../../client/common/platform/types'; +import * as processApi from '../../../client/common/process/rawProcessApis'; suite('Terminal Deactivation Limitation Prompt', () => { let shell: IApplicationShell; @@ -29,19 +27,37 @@ suite('Terminal Deactivation Limitation Prompt', () => { let deactivatePrompt: TerminalDeactivateLimitationPrompt; let terminalWriteEvent: EventEmitter; let notificationEnabled: IPersistentState; - let browserService: IBrowserService; let interpreterService: IInterpreterService; - const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain]; - const expectedMessage = Interpreters.terminalDeactivatePrompt; + let documentManager: IDocumentManager; + let fs: IFileSystem; + const prompts = [Common.editSomething.format('~/.bashrc'), Common.doNotShowAgain]; + const expectedMessage = Interpreters.terminalDeactivatePrompt.format('~/.bashrc'); setup(async () => { + const activeEditorEvent = new EventEmitter(); + const document = ({ + uri: Uri.file(''), + getText: () => '', + } as unknown) as TextDocument; + sinon.stub(processApi, 'shellExec').callsFake(async (command: string) => { + if (command !== 'code ~/.bashrc') { + throw new Error(`Unexpected command: ${command}`); + } + sleep(1500).then(() => { + activeEditorEvent.fire(undefined); + activeEditorEvent.fire({ document } as TextEditor); + }); + return { stdout: '' }; + }); + fs = mock(); + documentManager = mock(); + when(documentManager.onDidChangeActiveTextEditor).thenReturn(activeEditorEvent.event); shell = mock(); interpreterService = mock(); experimentService = mock(); persistentStateFactory = mock(); appEnvironment = mock(); when(appEnvironment.shell).thenReturn('bash'); - browserService = mock(); notificationEnabled = mock>(); terminalWriteEvent = new EventEmitter(); when(persistentStateFactory.createGlobalPersistentState(anything(), true)).thenReturn( @@ -54,8 +70,9 @@ suite('Terminal Deactivation Limitation Prompt', () => { instance(persistentStateFactory), [], instance(interpreterService), - instance(browserService), instance(appEnvironment), + instance(fs), + instance(documentManager), instance(experimentService), ); }); @@ -99,7 +116,7 @@ suite('Terminal Deactivation Limitation Prompt', () => { terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); await sleep(1); - verify(shell.showWarningMessage(expectedMessage, ...prompts)).once(); + verify(shell.showWarningMessage(expectedMessage, ...prompts)).never(); }); test('When not in experiment, do not show notification for the same', async () => { @@ -184,47 +201,25 @@ suite('Terminal Deactivation Limitation Prompt', () => { verify(notificationEnabled.updateValue(false)).once(); }); - test('Disable notification if `Done, it works` is clicked', async () => { - const resource = Uri.file('a'); - const terminal = ({ - creationOptions: { - cwd: resource, - }, - } as unknown) as Terminal; - when(notificationEnabled.value).thenReturn(true); - when(interpreterService.getActiveInterpreter(anything())).thenResolve(({ - type: PythonEnvType.Virtual, - } as unknown) as PythonEnvironment); - when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn( - Promise.resolve(Interpreters.deactivateDoneButton), - ); - - await deactivatePrompt.activate(); - terminalWriteEvent.fire({ data: 'Please deactivate me', terminal }); - await sleep(1); - - verify(notificationEnabled.updateValue(false)).once(); - }); - - test('Open link to workaround if `See instructions` is clicked', async () => { - const resource = Uri.file('a'); - const terminal = ({ - creationOptions: { - cwd: resource, - }, - } as unknown) as Terminal; + test('Edit script correctly if `Edit