Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Adding PYTHONSTARTUP with shell integration to environment variable collection #24111

Merged
merged 18 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,12 @@
"scope": "resource",
"type": "array"
},
"python.REPL.enableShellIntegration": {
"default": true,
"description": "%python.REPL.enableShellIntegration.description%",
"scope": "resource",
"type": "boolean"
},
"python.REPL.enableREPLSmartSend": {
"default": true,
"description": "%python.EnableREPLSmartSend.description%",
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"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`.",
Expand Down
1 change: 1 addition & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ export interface ITerminalSettings {
export interface IREPLSettings {
readonly enableREPLSmartSend: boolean;
readonly sendToNativeREPL: boolean;
readonly enableShellIntegration: boolean;
}

export interface IExperiments {
Expand Down
8 changes: 8 additions & 0 deletions src/client/common/vscodeApis/workspaceApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
return vscode.workspace.fs.createDirectory(uri);
}

export function copy(source: vscode.Uri, dest: vscode.Uri, options?: { overwrite?: boolean }): Thenable<void> {
return vscode.workspace.fs.copy(source, dest, options);
}
3 changes: 3 additions & 0 deletions src/client/extensionActivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,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.
Expand Down Expand Up @@ -177,6 +178,8 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch):

serviceManager.get<ITerminalAutoActivation>(ITerminalAutoActivation).register();

await registerPythonStartup(ext.context);

serviceManager.get<ICodeExecutionManager>(ICodeExecutionManager).registerCommands();

disposables.push(new ReplProvider(serviceContainer));
Expand Down
17 changes: 11 additions & 6 deletions src/client/terminals/envCollectionActivation/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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()
Expand Down Expand Up @@ -80,7 +84,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 shellIntegrationDetectionService: IShellIntegrationDetectionService,
@inject(IEnvironmentVariablesProvider)
private readonly environmentVariablesProvider: IEnvironmentVariablesProvider,
) {
Expand Down Expand Up @@ -113,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();
Expand All @@ -139,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(
Expand Down Expand Up @@ -328,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;
Expand Down Expand Up @@ -395,7 +400,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
}

private async getPrependOptions(): Promise<EnvironmentVariableMutatorOptions> {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -33,7 +33,7 @@ export enum isShellIntegrationWorking {
}

@injectable()
export class ShellIntegrationService implements IShellIntegrationService {
export class ShellIntegrationDetectionService implements IShellIntegrationDetectionService {
private isWorkingForShell = new Set<TerminalShellType>();

private readonly didChange = new EventEmitter<void>();
Expand Down
27 changes: 27 additions & 0 deletions src/client/terminals/pythonStartup.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
const config = getConfiguration('python');
const pythonrcSetting = config.get<boolean>('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');
}
}
10 changes: 7 additions & 3 deletions src/client/terminals/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ import {
ICodeExecutionHelper,
ICodeExecutionManager,
ICodeExecutionService,
IShellIntegrationService,
IShellIntegrationDetectionService,
ITerminalAutoActivation,
ITerminalDeactivateService,
ITerminalEnvVarCollectionService,
} from './types';
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';

export function registerTypes(serviceManager: IServiceManager): void {
serviceManager.addSingleton<ICodeExecutionHelper>(ICodeExecutionHelper, CodeExecutionHelper);
Expand Down Expand Up @@ -50,6 +50,10 @@ export function registerTypes(serviceManager: IServiceManager): void {
IExtensionSingleActivationService,
TerminalIndicatorPrompt,
);
serviceManager.addSingleton<IShellIntegrationService>(IShellIntegrationService, ShellIntegrationService);
serviceManager.addSingleton<IShellIntegrationDetectionService>(
IShellIntegrationDetectionService,
ShellIntegrationDetectionService,
);

serviceManager.addBinding(ITerminalEnvVarCollectionService, IExtensionActivationService);
}
9 changes: 7 additions & 2 deletions src/client/terminals/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
isWorking(): Promise<boolean>;
}
Expand All @@ -53,3 +53,8 @@ export interface ITerminalDeactivateService {
initializeScriptParams(shell: string): Promise<void>;
getScriptLocation(shell: string, resource: Resource): Promise<string | undefined>;
}

export const IPythonStartupEnvVarService = Symbol('IPythonStartupEnvVarService');
export interface IPythonStartupEnvVarService {
register(): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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()];
Expand All @@ -76,7 +76,7 @@ suite('Terminal Environment Variable Collection Service', () => {
context = mock<IExtensionContext>();
shell = mock<IApplicationShell>();
const envVarProvider = mock<IEnvironmentVariablesProvider>();
shellIntegrationService = mock<IShellIntegrationService>();
shellIntegrationService = mock<IShellIntegrationDetectionService>();
when(shellIntegrationService.isWorking()).thenResolve(true);
globalCollection = mock<GlobalEnvironmentVariableCollection>();
collection = mock<EnvironmentVariableCollection>();
Expand Down
7 changes: 6 additions & 1 deletion src/test/terminals/codeExecution/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
7 changes: 6 additions & 1 deletion src/test/terminals/codeExecution/smartSend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions src/test/terminals/serviceRegistry.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ 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,
} from '../../client/terminals/types';
import { ShellIntegrationDetectionService } from '../../client/terminals/envCollectionActivation/shellIntegrationService';

suite('Terminal - Service Registry', () => {
test('Ensure all services get registered', () => {
Expand All @@ -39,7 +39,7 @@ suite('Terminal - Service Registry', () => {
[ITerminalEnvVarCollectionService, TerminalEnvVarCollectionService],
[IExtensionSingleActivationService, TerminalIndicatorPrompt],
[ITerminalDeactivateService, TerminalDeactivateService],
[IShellIntegrationService, ShellIntegrationService],
[IShellIntegrationDetectionService, ShellIntegrationDetectionService],
].forEach((args) => {
if (args.length === 2) {
services
Expand Down
Loading