From e8dc28cc431b77d6eabb9d30e4584c3e7a1750ef Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 12:59:38 -0700 Subject: [PATCH 01/18] enable SI when typing Python in terminal --- src/client/common/terminal/service.ts | 4 +++ src/client/extensionActivation.ts | 9 ++++- .../envCollectionActivation/service.ts | 28 ++++++++++----- .../shellIntegrationService.ts | 4 +-- .../terminals/pythonStartupEnvVar/service.ts | 35 +++++++++++++++++++ src/client/terminals/serviceRegistry.ts | 14 ++++++-- src/client/terminals/types.ts | 9 +++-- ...rminalEnvVarCollectionService.unit.test.ts | 6 ++-- .../terminals/serviceRegistry.unit.test.ts | 6 ++-- 9 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 src/client/terminals/pythonStartupEnvVar/service.ts diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 19cdf0aea0a1..5d67188d1d1f 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -75,6 +75,7 @@ export class TerminalService implements ITerminalService, Disposable { } this.terminal!.sendText(text); } + public async executeCommand(commandLine: string): Promise { const terminal = this.terminal!; if (!this.options?.hideFromUser) { @@ -125,6 +126,9 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(preserveFocus); } } + // When terminal is launched + // Copy pythonrc into /pythonrc.py + // Update environment variable collection to include PYTHONSTARTUP=/pythonrc.py // TODO: Debt switch to Promise ---> breaks 20 tests public async ensureTerminal(preserveFocus: boolean = true): Promise { if (this.terminal) { diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index fe5d18a8b83f..fd63510c889c 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -32,7 +32,12 @@ import { TerminalProvider } from './providers/terminalProvider'; import { setExtensionInstallTelemetryProperties } from './telemetry/extensionInstallTelemetry'; import { registerTypes as tensorBoardRegisterTypes } from './tensorBoard/serviceRegistry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; -import { ICodeExecutionHelper, ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; +import { + ICodeExecutionHelper, + ICodeExecutionManager, + IPythonStartupEnvVarService, + ITerminalAutoActivation, +} from './terminals/types'; import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; // components @@ -177,6 +182,8 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): serviceManager.get(ITerminalAutoActivation).register(); + serviceManager.get(IPythonStartupEnvVarService).register(); + serviceManager.get(ICodeExecutionManager).registerCommands(); disposables.push(new ReplProvider(serviceContainer)); diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 77e478b3577d..11d05c611942 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -10,6 +10,7 @@ import { EnvironmentVariableScope, EnvironmentVariableMutatorOptions, ProgressLocation, + EnvironmentVariableCollection, } from 'vscode'; import { pathExists, normCase } from '../../common/platform/fs-paths'; import { IExtensionActivationService } from '../../activation/types'; @@ -37,7 +38,11 @@ import { TerminalShellType } from '../../common/terminal/types'; import { OSType } from '../../common/utils/platform'; import { PythonEnvType } from '../../pythonEnvironments/base/info'; -import { IShellIntegrationService, ITerminalDeactivateService, ITerminalEnvVarCollectionService } from '../types'; +import { + IShellIntegrationDetectionService, + ITerminalDeactivateService, + ITerminalEnvVarCollectionService, +} from '../types'; import { ProgressService } from '../../common/application/progressService'; @injectable() @@ -80,7 +85,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ITerminalDeactivateService) private readonly terminalDeactivateService: ITerminalDeactivateService, @inject(IPathUtils) private readonly pathUtils: IPathUtils, - @inject(IShellIntegrationService) private readonly shellIntegrationService: IShellIntegrationService, + @inject(IShellIntegrationDetectionService) + private readonly shellIntegrationService: IShellIntegrationDetectionService, @inject(IEnvironmentVariablesProvider) private readonly environmentVariablesProvider: IEnvironmentVariablesProvider, ) { @@ -91,7 +97,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async activate(resource: Resource): Promise { try { if (!inTerminalEnvVarExperiment(this.experimentService)) { - this.context.environmentVariableCollection.clear(); + clearCollectionVariables(this.context.environmentVariableCollection); await this.handleMicroVenv(resource); if (!this.registeredOnce) { this.interpreterService.onDidChangeInterpreter( @@ -171,7 +177,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const settings = this.configurationService.getSettings(resource); const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder }); if (!settings.terminal.activateEnvironment) { - envVarCollection.clear(); + clearCollectionVariables(envVarCollection); traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); return; } @@ -193,7 +199,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } await this.trackTerminalPrompt(shell, resource, env); - envVarCollection.clear(); + clearCollectionVariables(envVarCollection); this.processEnvVars = undefined; return; } @@ -210,7 +216,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const defaultPrependOptions = await this.getPrependOptions(); // Clear any previously set env vars from collection - envVarCollection.clear(); + clearCollectionVariables(envVarCollection); const deactivate = await this.terminalDeactivateService.getScriptLocation(shell, resource); Object.keys(env).forEach((key) => { if (shouldSkip(key)) { @@ -367,7 +373,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const settings = this.configurationService.getSettings(resource); const workspaceFolder = this.getWorkspaceFolder(resource); if (!settings.terminal.activateEnvironment) { - this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); + clearCollectionVariables(this.getEnvironmentVariableCollection({ workspaceFolder })); traceVerbose( 'Do not activate microvenv as activating environments in terminal is disabled for', resource?.fsPath, @@ -387,7 +393,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ ); return; } - this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); + clearCollectionVariables(this.getEnvironmentVariableCollection({ workspaceFolder })); } } catch (ex) { traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex); @@ -485,3 +491,9 @@ function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables { }); return result; } + +function clearCollectionVariables(collection: EnvironmentVariableCollection) { + for (const key of ['PS1', 'PATH']) { + collection.delete(key); + } +} diff --git a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts index 8ab3d84122b7..71cfb18dd437 100644 --- a/src/client/terminals/envCollectionActivation/shellIntegrationService.ts +++ b/src/client/terminals/envCollectionActivation/shellIntegrationService.ts @@ -14,7 +14,7 @@ import { TerminalShellType } from '../../common/terminal/types'; import { IDisposableRegistry, IPersistentStateFactory } from '../../common/types'; import { sleep } from '../../common/utils/async'; import { traceError, traceVerbose } from '../../logging'; -import { IShellIntegrationService } from '../types'; +import { IShellIntegrationDetectionService } from '../types'; /** * This is a list of shells which support shell integration: @@ -33,7 +33,7 @@ export enum isShellIntegrationWorking { } @injectable() -export class ShellIntegrationService implements IShellIntegrationService { +export class ShellIntegrationDetectionService implements IShellIntegrationDetectionService { private isWorkingForShell = new Set(); private readonly didChange = new EventEmitter(); diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts new file mode 100644 index 000000000000..db2efdbad499 --- /dev/null +++ b/src/client/terminals/pythonStartupEnvVar/service.ts @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { injectable, inject } from 'inversify'; +import { Uri, workspace } from 'vscode'; +import { IPythonStartupEnvVarService } from '../types'; +import { IExtensionContext } from '../../common/types'; +import { EXTENSION_ROOT_DIR } from '../../constants'; + +@injectable() +export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { + constructor(@inject(IExtensionContext) private context: IExtensionContext) {} + + public async register(): Promise { + const storageUri = this.context.storageUri || this.context.globalStorageUri; + try { + await workspace.fs.createDirectory(storageUri); + } catch { + // already exists, most likely + } + const destPath = Uri.joinPath(storageUri, 'pythonrc.py'); + + // TODO: Only do this when we have a setting + // Rollout strategy: + // Stage 1. Opt-in setting in stable/insiders + // Stage 2. Out-out setting in insiders + // Stage 3. Out-out setting in stable (or experiment?) + const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); + + await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); + + this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); + } +} diff --git a/src/client/terminals/serviceRegistry.ts b/src/client/terminals/serviceRegistry.ts index 3474edadd744..df86878bf546 100644 --- a/src/client/terminals/serviceRegistry.ts +++ b/src/client/terminals/serviceRegistry.ts @@ -12,7 +12,8 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, - IShellIntegrationService, + IPythonStartupEnvVarService, + IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, ITerminalEnvVarCollectionService, @@ -20,8 +21,10 @@ import { import { TerminalEnvVarCollectionService } from './envCollectionActivation/service'; import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types'; import { TerminalIndicatorPrompt } from './envCollectionActivation/indicatorPrompt'; -import { ShellIntegrationService } from './envCollectionActivation/shellIntegrationService'; + import { TerminalDeactivateService } from './envCollectionActivation/deactivateService'; +import { ShellIntegrationDetectionService } from './envCollectionActivation/shellIntegrationService'; +import { PythonStartupEnvVarService } from './pythonStartupEnvVar/service'; export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(ICodeExecutionHelper, CodeExecutionHelper); @@ -50,6 +53,11 @@ export function registerTypes(serviceManager: IServiceManager): void { IExtensionSingleActivationService, TerminalIndicatorPrompt, ); - serviceManager.addSingleton(IShellIntegrationService, ShellIntegrationService); + serviceManager.addSingleton( + IShellIntegrationDetectionService, + ShellIntegrationDetectionService, + ); + serviceManager.addSingleton(IPythonStartupEnvVarService, PythonStartupEnvVarService); + serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService); } diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 4c73da63dd1e..ada3acd851a9 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -42,8 +42,8 @@ export interface ITerminalEnvVarCollectionService { isTerminalPromptSetCorrectly(resource?: Resource): boolean; } -export const IShellIntegrationService = Symbol('IShellIntegrationService'); -export interface IShellIntegrationService { +export const IShellIntegrationDetectionService = Symbol('IShellIntegrationDetectionService'); +export interface IShellIntegrationDetectionService { onDidChangeStatus: Event; isWorking(): Promise; } @@ -53,3 +53,8 @@ export interface ITerminalDeactivateService { initializeScriptParams(shell: string): Promise; getScriptLocation(shell: string, resource: Resource): Promise; } + +export const IPythonStartupEnvVarService = Symbol('IPythonStartupEnvVarService'); +export interface IPythonStartupEnvVarService { + register(): void; +} diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 5d1027d12702..3550a92ba1ec 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -37,7 +37,7 @@ import { IInterpreterService } from '../../../client/interpreter/contracts'; import { PathUtils } from '../../../client/common/platform/pathUtils'; import { PythonEnvType } from '../../../client/pythonEnvironments/base/info'; import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; -import { IShellIntegrationService, ITerminalDeactivateService } from '../../../client/terminals/types'; +import { IShellIntegrationDetectionService, ITerminalDeactivateService } from '../../../client/terminals/types'; import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; suite('Terminal Environment Variable Collection Service', () => { @@ -58,7 +58,7 @@ suite('Terminal Environment Variable Collection Service', () => { title: Interpreters.activatingTerminals, }; let configService: IConfigurationService; - let shellIntegrationService: IShellIntegrationService; + let shellIntegrationService: IShellIntegrationDetectionService; const displayPath = 'display/path'; const customShell = 'powershell'; const defaultShell = defaultShells[getOSType()]; @@ -76,7 +76,7 @@ suite('Terminal Environment Variable Collection Service', () => { context = mock(); shell = mock(); const envVarProvider = mock(); - shellIntegrationService = mock(); + shellIntegrationService = mock(); when(shellIntegrationService.isWorking()).thenResolve(true); globalCollection = mock(); collection = mock(); diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index cb5b7715c4b7..7b8887cc66d3 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -14,13 +14,13 @@ import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecut import { TerminalDeactivateService } from '../../client/terminals/envCollectionActivation/deactivateService'; import { TerminalIndicatorPrompt } from '../../client/terminals/envCollectionActivation/indicatorPrompt'; import { TerminalEnvVarCollectionService } from '../../client/terminals/envCollectionActivation/service'; -import { ShellIntegrationService } from '../../client/terminals/envCollectionActivation/shellIntegrationService'; + import { registerTypes } from '../../client/terminals/serviceRegistry'; import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, - IShellIntegrationService, + IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, ITerminalEnvVarCollectionService, @@ -39,7 +39,7 @@ suite('Terminal - Service Registry', () => { [ITerminalEnvVarCollectionService, TerminalEnvVarCollectionService], [IExtensionSingleActivationService, TerminalIndicatorPrompt], [ITerminalDeactivateService, TerminalDeactivateService], - [IShellIntegrationService, ShellIntegrationService], + [IShellIntegrationDetectionService, IShellIntegrationDetectionService], ].forEach((args) => { if (args.length === 2) { services From a19d77c7e378b005652063cbdb0ea22f5ce60b2e Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 13:22:52 -0700 Subject: [PATCH 02/18] dont limit to clearing only PS1 and PATH --- .../envCollectionActivation/service.ts | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index 11d05c611942..f9aaccea363d 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -10,7 +10,6 @@ import { EnvironmentVariableScope, EnvironmentVariableMutatorOptions, ProgressLocation, - EnvironmentVariableCollection, } from 'vscode'; import { pathExists, normCase } from '../../common/platform/fs-paths'; import { IExtensionActivationService } from '../../activation/types'; @@ -97,7 +96,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async activate(resource: Resource): Promise { try { if (!inTerminalEnvVarExperiment(this.experimentService)) { - clearCollectionVariables(this.context.environmentVariableCollection); + this.context.environmentVariableCollection.clear(); await this.handleMicroVenv(resource); if (!this.registeredOnce) { this.interpreterService.onDidChangeInterpreter( @@ -177,7 +176,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const settings = this.configurationService.getSettings(resource); const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder }); if (!settings.terminal.activateEnvironment) { - clearCollectionVariables(envVarCollection); + envVarCollection.clear(); traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); return; } @@ -199,7 +198,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } await this.trackTerminalPrompt(shell, resource, env); - clearCollectionVariables(envVarCollection); + envVarCollection.clear(); this.processEnvVars = undefined; return; } @@ -216,7 +215,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const defaultPrependOptions = await this.getPrependOptions(); // Clear any previously set env vars from collection - clearCollectionVariables(envVarCollection); + envVarCollection.clear(); const deactivate = await this.terminalDeactivateService.getScriptLocation(shell, resource); Object.keys(env).forEach((key) => { if (shouldSkip(key)) { @@ -373,7 +372,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const settings = this.configurationService.getSettings(resource); const workspaceFolder = this.getWorkspaceFolder(resource); if (!settings.terminal.activateEnvironment) { - clearCollectionVariables(this.getEnvironmentVariableCollection({ workspaceFolder })); + this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); traceVerbose( 'Do not activate microvenv as activating environments in terminal is disabled for', resource?.fsPath, @@ -393,7 +392,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ ); return; } - clearCollectionVariables(this.getEnvironmentVariableCollection({ workspaceFolder })); + this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); } } catch (ex) { traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex); @@ -491,9 +490,3 @@ function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables { }); return result; } - -function clearCollectionVariables(collection: EnvironmentVariableCollection) { - for (const key of ['PS1', 'PATH']) { - collection.delete(key); - } -} From 4412586bb7b8de9f876647b5690308ac8caa1e7d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 13:54:19 -0700 Subject: [PATCH 03/18] add setting to toggle exporting of pythonstartup --- package.json | 6 ++++++ package.nls.json | 1 + src/client/common/configSettings.ts | 1 + src/client/common/types.ts | 1 + .../terminals/pythonStartupEnvVar/service.ts | 16 ++++++++++++---- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index ab0f4b1bb82d..42ee1b910208 100644 --- a/package.json +++ b/package.json @@ -631,6 +631,12 @@ "scope": "resource", "type": "array" }, + "python.terminal.enable Shell Integration in Python Terminal REPL": { + "default": true, + "description": "%python.terminal.enableSITerminal.description%", + "scope": "resource", + "type": "boolean" + }, "python.REPL.enableREPLSmartSend": { "default": true, "description": "%python.EnableREPLSmartSend.description%", diff --git a/package.nls.json b/package.nls.json index 5a5029231b17..f5e809027db2 100644 --- a/package.nls.json +++ b/package.nls.json @@ -68,6 +68,7 @@ "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", + "python.terminal.enableSITerminal.description": "Enable Shell Integration for Python Terminal REPL.", "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 6cae60c9fb97..ea9a27fb44b3 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -368,6 +368,7 @@ export class PythonSettings implements IPythonSettings { launchArgs: [], activateEnvironment: true, activateEnvInCurrentTerminal: false, + pythonrcStartup: true, }; this.REPL = pythonSettings.get('REPL')!; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 754e08004213..ad7316a5f6a6 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -197,6 +197,7 @@ export interface ITerminalSettings { readonly launchArgs: string[]; readonly activateEnvironment: boolean; readonly activateEnvInCurrentTerminal: boolean; + readonly pythonrcStartup: boolean; } export interface IREPLSettings { diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts index db2efdbad499..4446cb0b9f68 100644 --- a/src/client/terminals/pythonStartupEnvVar/service.ts +++ b/src/client/terminals/pythonStartupEnvVar/service.ts @@ -5,12 +5,15 @@ import * as path from 'path'; import { injectable, inject } from 'inversify'; import { Uri, workspace } from 'vscode'; import { IPythonStartupEnvVarService } from '../types'; -import { IExtensionContext } from '../../common/types'; +import { IConfigurationService, IExtensionContext } from '../../common/types'; import { EXTENSION_ROOT_DIR } from '../../constants'; @injectable() export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { - constructor(@inject(IExtensionContext) private context: IExtensionContext) {} + constructor( + @inject(IExtensionContext) private context: IExtensionContext, + @inject(IConfigurationService) private configurationService: IConfigurationService, + ) {} public async register(): Promise { const storageUri = this.context.storageUri || this.context.globalStorageUri; @@ -29,7 +32,12 @@ export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); - - this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); + const pythonrcSetting = this.configurationService.getSettings().terminal.pythonrcStartup; + if (pythonrcSetting) { + this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); + } else { + this.context.environmentVariableCollection.delete('PYTHONSTARTUP'); + } + // this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); } } From 4c663c35d4c33eddad55d95dde68e48ce6c1dfda Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 13:55:01 -0700 Subject: [PATCH 04/18] comment --- src/client/terminals/pythonStartupEnvVar/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts index 4446cb0b9f68..557bbcfe35a9 100644 --- a/src/client/terminals/pythonStartupEnvVar/service.ts +++ b/src/client/terminals/pythonStartupEnvVar/service.ts @@ -33,11 +33,11 @@ export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); const pythonrcSetting = this.configurationService.getSettings().terminal.pythonrcStartup; + // TODO: Is there better place to set this? if (pythonrcSetting) { this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); } else { this.context.environmentVariableCollection.delete('PYTHONSTARTUP'); } - // this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); } } From 02f4dc45fd00d2d10a9f3d1d00dc7a907b62d400 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 13:59:14 -0700 Subject: [PATCH 05/18] always clean up --- src/client/terminals/serviceRegistry.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/terminals/serviceRegistry.ts b/src/client/terminals/serviceRegistry.ts index df86878bf546..a77b3fc4150d 100644 --- a/src/client/terminals/serviceRegistry.ts +++ b/src/client/terminals/serviceRegistry.ts @@ -21,7 +21,6 @@ import { import { TerminalEnvVarCollectionService } from './envCollectionActivation/service'; import { IExtensionActivationService, IExtensionSingleActivationService } from '../activation/types'; import { TerminalIndicatorPrompt } from './envCollectionActivation/indicatorPrompt'; - import { TerminalDeactivateService } from './envCollectionActivation/deactivateService'; import { ShellIntegrationDetectionService } from './envCollectionActivation/shellIntegrationService'; import { PythonStartupEnvVarService } from './pythonStartupEnvVar/service'; From 677b620a874169cea5bbfc22c61359bbe7fe30bf Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 14:10:02 -0700 Subject: [PATCH 06/18] ShellIntegrationDetectionService not IShellIntegrationDetectionService unit test --- src/test/terminals/serviceRegistry.unit.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index 7b8887cc66d3..64b6b622c808 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -20,11 +20,13 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, + IPythonStartupEnvVarService, IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, ITerminalEnvVarCollectionService, } from '../../client/terminals/types'; +import { ShellIntegrationDetectionService } from '../../client/terminals/envCollectionActivation/shellIntegrationService'; suite('Terminal - Service Registry', () => { test('Ensure all services get registered', () => { @@ -39,7 +41,7 @@ suite('Terminal - Service Registry', () => { [ITerminalEnvVarCollectionService, TerminalEnvVarCollectionService], [IExtensionSingleActivationService, TerminalIndicatorPrompt], [ITerminalDeactivateService, TerminalDeactivateService], - [IShellIntegrationDetectionService, IShellIntegrationDetectionService], + [IShellIntegrationDetectionService, ShellIntegrationDetectionService], ].forEach((args) => { if (args.length === 2) { services From d85a4c0abe672cb33c87aeee5800f470876a23ea Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 14:11:28 -0700 Subject: [PATCH 07/18] unused --- src/test/terminals/serviceRegistry.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index 64b6b622c808..d36a6b52b7f5 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -20,7 +20,6 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, - IPythonStartupEnvVarService, IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, From 3b9f82d2a980a144bfdb7455640cf886c9b20a05 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 14:20:57 -0700 Subject: [PATCH 08/18] IPythonStartupEnvVarService to serviceRegistry test --- src/test/terminals/serviceRegistry.unit.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index d36a6b52b7f5..9a97aafd54de 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -20,12 +20,14 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, + IPythonStartupEnvVarService, IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, ITerminalEnvVarCollectionService, } from '../../client/terminals/types'; import { ShellIntegrationDetectionService } from '../../client/terminals/envCollectionActivation/shellIntegrationService'; +import { PythonStartupEnvVarService } from '../../client/terminals/pythonStartupEnvVar/service'; suite('Terminal - Service Registry', () => { test('Ensure all services get registered', () => { @@ -41,6 +43,7 @@ suite('Terminal - Service Registry', () => { [IExtensionSingleActivationService, TerminalIndicatorPrompt], [ITerminalDeactivateService, TerminalDeactivateService], [IShellIntegrationDetectionService, ShellIntegrationDetectionService], + [IPythonStartupEnvVarService, PythonStartupEnvVarService], ].forEach((args) => { if (args.length === 2) { services From 99fc20cf020d912a73578117653681dbfe20d2e4 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Sun, 15 Sep 2024 21:52:50 -0700 Subject: [PATCH 09/18] remove comment --- src/client/terminals/pythonStartupEnvVar/service.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts index 557bbcfe35a9..2898373f466b 100644 --- a/src/client/terminals/pythonStartupEnvVar/service.ts +++ b/src/client/terminals/pythonStartupEnvVar/service.ts @@ -24,11 +24,6 @@ export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { } const destPath = Uri.joinPath(storageUri, 'pythonrc.py'); - // TODO: Only do this when we have a setting - // Rollout strategy: - // Stage 1. Opt-in setting in stable/insiders - // Stage 2. Out-out setting in insiders - // Stage 3. Out-out setting in stable (or experiment?) const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); From 8e8dd187b55954a702e4cc929da38d0d43d350d0 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 16 Sep 2024 10:57:34 -0700 Subject: [PATCH 10/18] better name better description --- package.json | 4 ++-- package.nls.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 42ee1b910208..bec7568cbd47 100644 --- a/package.json +++ b/package.json @@ -631,9 +631,9 @@ "scope": "resource", "type": "array" }, - "python.terminal.enable Shell Integration in Python Terminal REPL": { + "python.terminal.pythonStartupForShellIntegration": { "default": true, - "description": "%python.terminal.enableSITerminal.description%", + "description": "%python.terminal.pythonStartupForShellIntegration.description%", "scope": "resource", "type": "boolean" }, diff --git a/package.nls.json b/package.nls.json index f5e809027db2..3e539327d8f2 100644 --- a/package.nls.json +++ b/package.nls.json @@ -68,7 +68,7 @@ "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", - "python.terminal.enableSITerminal.description": "Enable Shell Integration for Python Terminal REPL.", + "python.terminal.pythonStartupForShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration allows command decorations, run recent command, and better accessibility for Python REPL in the terminal.", "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", From 7b7183cda26562ccd0f170f7fb7cdd24f8126a80 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Tue, 17 Sep 2024 02:05:10 -0400 Subject: [PATCH 11/18] Update package.nls.json Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com> --- package.nls.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.nls.json b/package.nls.json index 3e539327d8f2..77af80cecb2c 100644 --- a/package.nls.json +++ b/package.nls.json @@ -68,7 +68,7 @@ "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", - "python.terminal.pythonStartupForShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration allows command decorations, run recent command, and better accessibility for Python REPL in the terminal.", + "python.terminal.pythonStartupForShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration enhances the terminal experience by allowing command decorations, run recent command, and improving accessibility for Python REPL in the terminal." "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", From 327662e58b7f9ad5e184acea00c9bd078ebabcc6 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:27:57 -0700 Subject: [PATCH 12/18] rename to python repl enableShellIntegration --- package.json | 4 ++-- package.nls.json | 2 +- src/client/common/types.ts | 2 +- src/client/terminals/pythonStartupEnvVar/service.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index bec7568cbd47..c67cab273518 100644 --- a/package.json +++ b/package.json @@ -631,9 +631,9 @@ "scope": "resource", "type": "array" }, - "python.terminal.pythonStartupForShellIntegration": { + "python.REPL.enableShellIntegration": { "default": true, - "description": "%python.terminal.pythonStartupForShellIntegration.description%", + "description": "%python.terminal.enableShellIntegration.description%", "scope": "resource", "type": "boolean" }, diff --git a/package.nls.json b/package.nls.json index 77af80cecb2c..483699fddfc6 100644 --- a/package.nls.json +++ b/package.nls.json @@ -68,7 +68,7 @@ "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", - "python.terminal.pythonStartupForShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration enhances the terminal experience by allowing command decorations, run recent command, and improving accessibility for Python REPL in the terminal." + "python.terminal.enableShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration enhances the terminal experience by allowing command decorations, run recent command, and improving accessibility for Python REPL in the terminal.", "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", diff --git a/src/client/common/types.ts b/src/client/common/types.ts index ad7316a5f6a6..283319fd6cec 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -197,12 +197,12 @@ export interface ITerminalSettings { readonly launchArgs: string[]; readonly activateEnvironment: boolean; readonly activateEnvInCurrentTerminal: boolean; - readonly pythonrcStartup: boolean; } export interface IREPLSettings { readonly enableREPLSmartSend: boolean; readonly sendToNativeREPL: boolean; + readonly enableShellIntegration: boolean; } export interface IExperiments { diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts index 2898373f466b..378a36be8d2b 100644 --- a/src/client/terminals/pythonStartupEnvVar/service.ts +++ b/src/client/terminals/pythonStartupEnvVar/service.ts @@ -27,7 +27,7 @@ export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); - const pythonrcSetting = this.configurationService.getSettings().terminal.pythonrcStartup; + const pythonrcSetting = this.configurationService.getSettings().REPL.enableShellIntegration; // TODO: Is there better place to set this? if (pythonrcSetting) { this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); From 6f544bf035a28f6edec4a585ef3c595e0da1a129 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:31:47 -0700 Subject: [PATCH 13/18] renaming legacy shellIntegrationService to shellIntegrationDetectionService --- .../terminals/envCollectionActivation/service.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index f9aaccea363d..62971aa1fa98 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -85,7 +85,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ @inject(ITerminalDeactivateService) private readonly terminalDeactivateService: ITerminalDeactivateService, @inject(IPathUtils) private readonly pathUtils: IPathUtils, @inject(IShellIntegrationDetectionService) - private readonly shellIntegrationService: IShellIntegrationDetectionService, + private readonly shellIntegrationDetectionService: IShellIntegrationDetectionService, @inject(IEnvironmentVariablesProvider) private readonly environmentVariablesProvider: IEnvironmentVariablesProvider, ) { @@ -118,7 +118,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this, this.disposables, ); - this.shellIntegrationService.onDidChangeStatus( + this.shellIntegrationDetectionService.onDidChangeStatus( async () => { traceInfo("Shell integration status changed, can confirm it's working."); await this._applyCollection(undefined).ignoreErrors(); @@ -144,7 +144,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ this.disposables, ); const { shell } = this.applicationEnvironment; - const isActive = await this.shellIntegrationService.isWorking(); + const isActive = await this.shellIntegrationDetectionService.isWorking(); const shellType = identifyShellFromShellPath(shell); if (!isActive && shellType !== TerminalShellType.commandPrompt) { traceWarn( @@ -333,7 +333,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // PS1 should be set but no PS1 was set. return; } - const config = await this.shellIntegrationService.isWorking(); + const config = await this.shellIntegrationDetectionService.isWorking(); if (!config) { traceVerbose('PS1 is not set when shell integration is disabled.'); return; @@ -400,7 +400,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ } private async getPrependOptions(): Promise { - const isActive = await this.shellIntegrationService.isWorking(); + const isActive = await this.shellIntegrationDetectionService.isWorking(); // 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 From 44b156ebcbd5cd11fd4587174637f009c36678e5 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:33:28 -0700 Subject: [PATCH 14/18] fix test to add enableShellIntegration field to make test happy --- src/client/common/configSettings.ts | 1 - src/test/terminals/codeExecution/helper.test.ts | 7 ++++++- src/test/terminals/codeExecution/smartSend.test.ts | 7 ++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index ea9a27fb44b3..6cae60c9fb97 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -368,7 +368,6 @@ export class PythonSettings implements IPythonSettings { launchArgs: [], activateEnvironment: true, activateEnvInCurrentTerminal: false, - pythonrcStartup: true, }; this.REPL = pythonSettings.get('REPL')!; diff --git a/src/test/terminals/codeExecution/helper.test.ts b/src/test/terminals/codeExecution/helper.test.ts index 9a6deefcb7bf..e15c41957726 100644 --- a/src/test/terminals/codeExecution/helper.test.ts +++ b/src/test/terminals/codeExecution/helper.test.ts @@ -112,7 +112,12 @@ suite('Terminal - Code Execution Helper', () => { activeResourceService.setup((a) => a.getActiveResource()).returns(() => resource); pythonSettings .setup((s) => s.REPL) - .returns(() => ({ enableREPLSmartSend: false, REPLSmartSend: false, sendToNativeREPL: false })); + .returns(() => ({ + enableREPLSmartSend: false, + REPLSmartSend: false, + sendToNativeREPL: false, + enableShellIntegration: true, + })); configurationService.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); configurationService .setup((c) => c.getSettings(TypeMoq.It.isAny())) diff --git a/src/test/terminals/codeExecution/smartSend.test.ts b/src/test/terminals/codeExecution/smartSend.test.ts index 89f5ac2b5e4d..594db361f51e 100644 --- a/src/test/terminals/codeExecution/smartSend.test.ts +++ b/src/test/terminals/codeExecution/smartSend.test.ts @@ -109,7 +109,12 @@ suite('REPL - Smart Send', () => { pythonSettings .setup((s) => s.REPL) - .returns(() => ({ enableREPLSmartSend: true, REPLSmartSend: true, sendToNativeREPL: false })); + .returns(() => ({ + enableREPLSmartSend: true, + REPLSmartSend: true, + sendToNativeREPL: false, + enableShellIntegration: true, + })); configurationService.setup((x) => x.getSettings(TypeMoq.It.isAny())).returns(() => pythonSettings.object); From 7bdc4b5da0f1f2a2f691661efc054880e125a67d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:38:21 -0700 Subject: [PATCH 15/18] remove unused comment --- src/client/common/terminal/service.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 5d67188d1d1f..19cdf0aea0a1 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -75,7 +75,6 @@ export class TerminalService implements ITerminalService, Disposable { } this.terminal!.sendText(text); } - public async executeCommand(commandLine: string): Promise { const terminal = this.terminal!; if (!this.options?.hideFromUser) { @@ -126,9 +125,6 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(preserveFocus); } } - // When terminal is launched - // Copy pythonrc into /pythonrc.py - // Update environment variable collection to include PYTHONSTARTUP=/pythonrc.py // TODO: Debt switch to Promise ---> breaks 20 tests public async ensureTerminal(preserveFocus: boolean = true): Promise { if (this.terminal) { From a50490fd194a1bb7c8c7b86ffa32db9d8b9a6519 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:39:16 -0700 Subject: [PATCH 16/18] clean up --- src/test/terminals/serviceRegistry.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index 9a97aafd54de..485b1b39c46b 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -14,7 +14,6 @@ import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecut import { TerminalDeactivateService } from '../../client/terminals/envCollectionActivation/deactivateService'; import { TerminalIndicatorPrompt } from '../../client/terminals/envCollectionActivation/indicatorPrompt'; import { TerminalEnvVarCollectionService } from '../../client/terminals/envCollectionActivation/service'; - import { registerTypes } from '../../client/terminals/serviceRegistry'; import { ICodeExecutionHelper, From 76a6a480921fc7301a9bb6d61e2e357131038672 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:41:47 -0700 Subject: [PATCH 17/18] rename nls description properly to repl.enableShellIntegration --- package.json | 2 +- package.nls.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index c67cab273518..1b433af12199 100644 --- a/package.json +++ b/package.json @@ -633,7 +633,7 @@ }, "python.REPL.enableShellIntegration": { "default": true, - "description": "%python.terminal.enableShellIntegration.description%", + "description": "%python.REPL.enableShellIntegration.description%", "scope": "resource", "type": "boolean" }, diff --git a/package.nls.json b/package.nls.json index 483699fddfc6..f032f3d7c275 100644 --- a/package.nls.json +++ b/package.nls.json @@ -65,10 +65,10 @@ "python.pixiToolPath.description": "Path to the pixi executable.", "python.EnableREPLSmartSend.description": "Toggle Smart Send for the Python REPL. Smart Send enables sending the smallest runnable block of code to the REPL on Shift+Enter and moves the cursor accordingly.", "python.REPL.sendToNativeREPL.description": "Toggle to send code to Python REPL instead of the terminal on execution. Turning this on will change the behavior for both Smart Send and Run Selection/Line in the Context Menu.", + "python.REPL.enableShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration enhances the terminal experience by allowing command decorations, run recent command, and improving accessibility for Python REPL in the terminal.", "python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.", "python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.", "python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.", - "python.terminal.enableShellIntegration.description": "Enable Shell Integration for Python Terminal REPL. Shell Integration enhances the terminal experience by allowing command decorations, run recent command, and improving accessibility for Python REPL in the terminal.", "python.terminal.activateEnvInCurrentTerminal.description": "Activate Python Environment in the current Terminal on load of the Extension.", "python.terminal.activateEnvironment.description": "Activate Python Environment in all Terminals created.", "python.terminal.executeInFileDir.description": "When executing a file in the terminal, whether to use execute in the file's directory, instead of the current open folder.", From 1cd461aa50289287fe04d23f35794eb5a13811c0 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 18 Sep 2024 11:16:47 -0700 Subject: [PATCH 18/18] separate out pythonStartup as a function not class --- src/client/common/vscodeApis/workspaceApis.ts | 8 ++++ src/client/extensionActivation.ts | 10 ++--- src/client/terminals/pythonStartup.ts | 27 +++++++++++++ .../terminals/pythonStartupEnvVar/service.ts | 38 ------------------- src/client/terminals/serviceRegistry.ts | 3 -- .../terminals/serviceRegistry.unit.test.ts | 3 -- 6 files changed, 38 insertions(+), 51 deletions(-) create mode 100644 src/client/terminals/pythonStartup.ts delete mode 100644 src/client/terminals/pythonStartupEnvVar/service.ts diff --git a/src/client/common/vscodeApis/workspaceApis.ts b/src/client/common/vscodeApis/workspaceApis.ts index 137382dc71a0..cb516da73075 100644 --- a/src/client/common/vscodeApis/workspaceApis.ts +++ b/src/client/common/vscodeApis/workspaceApis.ts @@ -93,3 +93,11 @@ export function isTrusted(): boolean { export function onDidGrantWorkspaceTrust(handler: () => void): vscode.Disposable { return vscode.workspace.onDidGrantWorkspaceTrust(handler); } + +export function createDirectory(uri: vscode.Uri): Thenable { + return vscode.workspace.fs.createDirectory(uri); +} + +export function copy(source: vscode.Uri, dest: vscode.Uri, options?: { overwrite?: boolean }): Thenable { + return vscode.workspace.fs.copy(source, dest, options); +} diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index fd63510c889c..429004e951cb 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -32,12 +32,7 @@ import { TerminalProvider } from './providers/terminalProvider'; import { setExtensionInstallTelemetryProperties } from './telemetry/extensionInstallTelemetry'; import { registerTypes as tensorBoardRegisterTypes } from './tensorBoard/serviceRegistry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; -import { - ICodeExecutionHelper, - ICodeExecutionManager, - IPythonStartupEnvVarService, - ITerminalAutoActivation, -} from './terminals/types'; +import { ICodeExecutionHelper, ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; // components @@ -59,6 +54,7 @@ import { DebuggerTypeName } from './debugger/constants'; import { StopWatch } from './common/utils/stopWatch'; import { registerReplCommands, registerReplExecuteOnEnter, registerStartNativeReplCommand } from './repl/replCommands'; import { registerTriggerForTerminalREPL } from './terminals/codeExecution/terminalReplWatcher'; +import { registerPythonStartup } from './terminals/pythonStartup'; export async function activateComponents( // `ext` is passed to any extra activation funcs. @@ -182,7 +178,7 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): serviceManager.get(ITerminalAutoActivation).register(); - serviceManager.get(IPythonStartupEnvVarService).register(); + await registerPythonStartup(ext.context); serviceManager.get(ICodeExecutionManager).registerCommands(); diff --git a/src/client/terminals/pythonStartup.ts b/src/client/terminals/pythonStartup.ts new file mode 100644 index 000000000000..9a6b956d7f6e --- /dev/null +++ b/src/client/terminals/pythonStartup.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ExtensionContext, Uri } from 'vscode'; +import * as path from 'path'; +import { copy, createDirectory, getConfiguration } from '../common/vscodeApis/workspaceApis'; +import { EXTENSION_ROOT_DIR } from '../constants'; + +export async function registerPythonStartup(context: ExtensionContext): Promise { + const config = getConfiguration('python'); + const pythonrcSetting = config.get('REPL.enableShellIntegration'); + + if (pythonrcSetting) { + const storageUri = context.storageUri || context.globalStorageUri; + try { + await createDirectory(storageUri); + } catch { + // already exists, most likely + } + const destPath = Uri.joinPath(storageUri, 'pythonrc.py'); + const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); + await copy(Uri.file(sourcePath), destPath, { overwrite: true }); + context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); + } else { + context.environmentVariableCollection.delete('PYTHONSTARTUP'); + } +} diff --git a/src/client/terminals/pythonStartupEnvVar/service.ts b/src/client/terminals/pythonStartupEnvVar/service.ts deleted file mode 100644 index 378a36be8d2b..000000000000 --- a/src/client/terminals/pythonStartupEnvVar/service.ts +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import * as path from 'path'; -import { injectable, inject } from 'inversify'; -import { Uri, workspace } from 'vscode'; -import { IPythonStartupEnvVarService } from '../types'; -import { IConfigurationService, IExtensionContext } from '../../common/types'; -import { EXTENSION_ROOT_DIR } from '../../constants'; - -@injectable() -export class PythonStartupEnvVarService implements IPythonStartupEnvVarService { - constructor( - @inject(IExtensionContext) private context: IExtensionContext, - @inject(IConfigurationService) private configurationService: IConfigurationService, - ) {} - - public async register(): Promise { - const storageUri = this.context.storageUri || this.context.globalStorageUri; - try { - await workspace.fs.createDirectory(storageUri); - } catch { - // already exists, most likely - } - const destPath = Uri.joinPath(storageUri, 'pythonrc.py'); - - const sourcePath = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); - - await workspace.fs.copy(Uri.file(sourcePath), destPath, { overwrite: true }); - const pythonrcSetting = this.configurationService.getSettings().REPL.enableShellIntegration; - // TODO: Is there better place to set this? - if (pythonrcSetting) { - this.context.environmentVariableCollection.replace('PYTHONSTARTUP', destPath.fsPath); - } else { - this.context.environmentVariableCollection.delete('PYTHONSTARTUP'); - } - } -} diff --git a/src/client/terminals/serviceRegistry.ts b/src/client/terminals/serviceRegistry.ts index a77b3fc4150d..e62701dcec0e 100644 --- a/src/client/terminals/serviceRegistry.ts +++ b/src/client/terminals/serviceRegistry.ts @@ -12,7 +12,6 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, - IPythonStartupEnvVarService, IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, @@ -23,7 +22,6 @@ import { IExtensionActivationService, IExtensionSingleActivationService } from ' import { TerminalIndicatorPrompt } from './envCollectionActivation/indicatorPrompt'; import { TerminalDeactivateService } from './envCollectionActivation/deactivateService'; import { ShellIntegrationDetectionService } from './envCollectionActivation/shellIntegrationService'; -import { PythonStartupEnvVarService } from './pythonStartupEnvVar/service'; export function registerTypes(serviceManager: IServiceManager): void { serviceManager.addSingleton(ICodeExecutionHelper, CodeExecutionHelper); @@ -56,7 +54,6 @@ export function registerTypes(serviceManager: IServiceManager): void { IShellIntegrationDetectionService, ShellIntegrationDetectionService, ); - serviceManager.addSingleton(IPythonStartupEnvVarService, PythonStartupEnvVarService); serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService); } diff --git a/src/test/terminals/serviceRegistry.unit.test.ts b/src/test/terminals/serviceRegistry.unit.test.ts index 485b1b39c46b..4f865cdedc0d 100644 --- a/src/test/terminals/serviceRegistry.unit.test.ts +++ b/src/test/terminals/serviceRegistry.unit.test.ts @@ -19,14 +19,12 @@ import { ICodeExecutionHelper, ICodeExecutionManager, ICodeExecutionService, - IPythonStartupEnvVarService, IShellIntegrationDetectionService, ITerminalAutoActivation, ITerminalDeactivateService, ITerminalEnvVarCollectionService, } from '../../client/terminals/types'; import { ShellIntegrationDetectionService } from '../../client/terminals/envCollectionActivation/shellIntegrationService'; -import { PythonStartupEnvVarService } from '../../client/terminals/pythonStartupEnvVar/service'; suite('Terminal - Service Registry', () => { test('Ensure all services get registered', () => { @@ -42,7 +40,6 @@ suite('Terminal - Service Registry', () => { [IExtensionSingleActivationService, TerminalIndicatorPrompt], [ITerminalDeactivateService, TerminalDeactivateService], [IShellIntegrationDetectionService, ShellIntegrationDetectionService], - [IPythonStartupEnvVarService, PythonStartupEnvVarService], ].forEach((args) => { if (args.length === 2) { services