From 4153f7c233b352771a0c4eb2d93c249e7192134f Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Tue, 3 Dec 2024 21:57:06 -0800 Subject: [PATCH] =?UTF-8?q?Fix:=20Don=E2=80=99t=20use=20executeCommand=20o?= =?UTF-8?q?n=20python=20sub-shell=20for=20windows.=20=20(#24542)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: https://github.com/microsoft/vscode-python/pull/24418 Further resolves: https://github.com/microsoft/vscode-python/issues/24422 Shell integration does not exist on windows python repl so dont use execute command on python subshell in this case. This will prevent keyboard interrupt from happening even when setting for shell integration is enabled for python shell (which is currently only supported on mac and linux) --- src/client/common/configSettings.ts | 3 +- src/client/common/pipes/namedPipes.ts | 2 +- src/client/common/platform/platformService.ts | 6 +--- src/client/common/serviceRegistry.ts | 2 +- src/client/common/terminal/service.ts | 3 +- src/client/common/utils/platform.ts | 4 +++ .../locators/common/nativePythonFinder.ts | 3 +- .../base/locators/common/pythonWatcher.ts | 2 +- .../common/environmentManagers/pixi.ts | 3 +- .../creation/common/commonUtils.ts | 2 +- .../creation/provider/venvUtils.ts | 2 +- src/test/common/configSettings.test.ts | 2 +- .../common/terminals/service.unit.test.ts | 28 ++++++++++++++++++- .../pythonEnvironments/nativeAPI.unit.test.ts | 3 +- src/test/serviceRegistry.ts | 3 +- 15 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 1b637e7aac2d..875e4b1baa6c 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -36,8 +36,7 @@ import { } from './types'; import { debounceSync } from './utils/decorators'; import { SystemVariables } from './variables/systemVariables'; -import { getOSType, OSType } from './utils/platform'; -import { isWindows } from './platform/platformService'; +import { getOSType, OSType, isWindows } from './utils/platform'; import { untildify } from './helpers'; export class PythonSettings implements IPythonSettings { diff --git a/src/client/common/pipes/namedPipes.ts b/src/client/common/pipes/namedPipes.ts index 81a2444f9bf0..9722b4bdcc58 100644 --- a/src/client/common/pipes/namedPipes.ts +++ b/src/client/common/pipes/namedPipes.ts @@ -10,7 +10,7 @@ import * as path from 'path'; import * as rpc from 'vscode-jsonrpc/node'; import { CancellationError, CancellationToken, Disposable } from 'vscode'; import { traceVerbose } from '../../logging'; -import { isWindows } from '../platform/platformService'; +import { isWindows } from '../utils/platform'; import { createDeferred } from '../utils/async'; const { XDG_RUNTIME_DIR } = process.env; diff --git a/src/client/common/platform/platformService.ts b/src/client/common/platform/platformService.ts index aa139eeeebc0..dc9b04cc652c 100644 --- a/src/client/common/platform/platformService.ts +++ b/src/client/common/platform/platformService.ts @@ -7,7 +7,7 @@ import { injectable } from 'inversify'; import * as os from 'os'; import { coerce, SemVer } from 'semver'; import { getSearchPathEnvVarNames } from '../utils/exec'; -import { Architecture, getArchitecture, getOSType, OSType } from '../utils/platform'; +import { Architecture, getArchitecture, getOSType, isWindows, OSType } from '../utils/platform'; import { parseSemVerSafe } from '../utils/version'; import { IPlatformService } from './types'; @@ -73,7 +73,3 @@ export class PlatformService implements IPlatformService { return getArchitecture() === Architecture.x64; } } - -export function isWindows(): boolean { - return getOSType() === OSType.Windows; -} diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 307d3ffe038f..5b9eb544f93b 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -88,7 +88,7 @@ import { Random } from './utils/random'; import { ContextKeyManager } from './application/contextKeyManager'; import { CreatePythonFileCommandHandler } from './application/commands/createPythonFile'; import { RequireJupyterPrompt } from '../jupyter/requireJupyterPrompt'; -import { isWindows } from './platform/platformService'; +import { isWindows } from './utils/platform'; import { PixiActivationCommandProvider } from './terminal/environmentActivationProviders/pixiActivationProvider'; export function registerTypes(serviceManager: IServiceManager): void { diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index d3a9652acb1f..e37539f1bc7c 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -21,6 +21,7 @@ import { } from './types'; import { traceVerbose } from '../../logging'; import { getConfiguration } from '../vscodeApis/workspaceApis'; +import { isWindows } from '../utils/platform'; @injectable() export class TerminalService implements ITerminalService, Disposable { @@ -104,7 +105,7 @@ export class TerminalService implements ITerminalService, Disposable { const config = getConfiguration('python'); const pythonrcSetting = config.get('terminal.shellIntegration.enabled'); - if (isPythonShell && !pythonrcSetting) { + if ((isPythonShell && !pythonrcSetting) || (isPythonShell && isWindows())) { // If user has explicitly disabled SI for Python, use sendText for inside Terminal REPL. terminal.sendText(commandLine); return undefined; diff --git a/src/client/common/utils/platform.ts b/src/client/common/utils/platform.ts index cf3b28e5cc35..c86f5ff9364e 100644 --- a/src/client/common/utils/platform.ts +++ b/src/client/common/utils/platform.ts @@ -67,3 +67,7 @@ export function getUserHomeDir(): string | undefined { } return getEnvironmentVariable('HOME') || getEnvironmentVariable('HOMEPATH'); } + +export function isWindows(): boolean { + return getOSType() === OSType.Windows; +} diff --git a/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts b/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts index a084cc1e4a16..cb5ab63077c9 100644 --- a/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts +++ b/src/client/pythonEnvironments/base/locators/common/nativePythonFinder.ts @@ -7,7 +7,7 @@ import * as path from 'path'; import * as rpc from 'vscode-jsonrpc/node'; import { PassThrough } from 'stream'; import * as fs from '../../../../common/platform/fs-paths'; -import { isWindows } from '../../../../common/platform/platformService'; +import { isWindows, getUserHomeDir } from '../../../../common/utils/platform'; import { EXTENSION_ROOT_DIR } from '../../../../constants'; import { createDeferred, createDeferredFrom } from '../../../../common/utils/async'; import { DisposableBase, DisposableStore } from '../../../../common/utils/resourceLifecycle'; @@ -15,7 +15,6 @@ import { noop } from '../../../../common/utils/misc'; import { getConfiguration, getWorkspaceFolderPaths, isTrusted } from '../../../../common/vscodeApis/workspaceApis'; import { CONDAPATH_SETTING_KEY } from '../../../common/environmentManagers/conda'; import { VENVFOLDERS_SETTING_KEY, VENVPATH_SETTING_KEY } from '../lowLevel/customVirtualEnvLocator'; -import { getUserHomeDir } from '../../../../common/utils/platform'; import { createLogOutputChannel } from '../../../../common/vscodeApis/windowApis'; import { sendNativeTelemetry, NativePythonTelemetry } from './nativePythonTelemetry'; import { NativePythonEnvironmentKind } from './nativePythonUtils'; diff --git a/src/client/pythonEnvironments/base/locators/common/pythonWatcher.ts b/src/client/pythonEnvironments/base/locators/common/pythonWatcher.ts index ce7851ec729f..378a0d6c521e 100644 --- a/src/client/pythonEnvironments/base/locators/common/pythonWatcher.ts +++ b/src/client/pythonEnvironments/base/locators/common/pythonWatcher.ts @@ -3,7 +3,7 @@ import { Disposable, Event, EventEmitter, GlobPattern, RelativePattern, Uri, WorkspaceFolder } from 'vscode'; import { createFileSystemWatcher, getWorkspaceFolder } from '../../../../common/vscodeApis/workspaceApis'; -import { isWindows } from '../../../../common/platform/platformService'; +import { isWindows } from '../../../../common/utils/platform'; import { arePathsSame } from '../../../common/externalDependencies'; import { FileChangeType } from '../../../../common/platform/fileSystemWatcher'; diff --git a/src/client/pythonEnvironments/common/environmentManagers/pixi.ts b/src/client/pythonEnvironments/common/environmentManagers/pixi.ts index 6abf26f830fb..9ad98d1714fb 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/pixi.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/pixi.ts @@ -6,12 +6,11 @@ import * as path from 'path'; import { readJSON } from 'fs-extra'; import which from 'which'; -import { getUserHomeDir } from '../../../common/utils/platform'; +import { getUserHomeDir, isWindows } from '../../../common/utils/platform'; import { exec, getPythonSetting, onDidChangePythonSetting, pathExists } from '../externalDependencies'; import { cache } from '../../../common/utils/decorators'; import { traceVerbose, traceWarn } from '../../../logging'; import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts'; -import { isWindows } from '../../../common/platform/platformService'; import { IDisposableRegistry } from '../../../common/types'; import { getWorkspaceFolderPaths } from '../../../common/vscodeApis/workspaceApis'; import { isTestExecution } from '../../../common/constants'; diff --git a/src/client/pythonEnvironments/creation/common/commonUtils.ts b/src/client/pythonEnvironments/creation/common/commonUtils.ts index 5d5ee8b0310a..8b6ffe1af450 100644 --- a/src/client/pythonEnvironments/creation/common/commonUtils.ts +++ b/src/client/pythonEnvironments/creation/common/commonUtils.ts @@ -7,7 +7,7 @@ import { Commands } from '../../../common/constants'; import { Common } from '../../../common/utils/localize'; import { executeCommand } from '../../../common/vscodeApis/commandApis'; import { showErrorMessage } from '../../../common/vscodeApis/windowApis'; -import { isWindows } from '../../../common/platform/platformService'; +import { isWindows } from '../../../common/utils/platform'; export async function showErrorMessageWithLogs(message: string): Promise { const result = await showErrorMessage(message, Common.openOutputPanel, Common.selectPythonInterpreter); diff --git a/src/client/pythonEnvironments/creation/provider/venvUtils.ts b/src/client/pythonEnvironments/creation/provider/venvUtils.ts index 5a2c0bb8a2d3..1bfb2c96f224 100644 --- a/src/client/pythonEnvironments/creation/provider/venvUtils.ts +++ b/src/client/pythonEnvironments/creation/provider/venvUtils.ts @@ -26,7 +26,7 @@ import { import { findFiles } from '../../../common/vscodeApis/workspaceApis'; import { traceError, traceVerbose } from '../../../logging'; import { Commands } from '../../../common/constants'; -import { isWindows } from '../../../common/platform/platformService'; +import { isWindows } from '../../../common/utils/platform'; import { getVenvPath, hasVenv } from '../common/commonUtils'; import { deleteEnvironmentNonWindows, deleteEnvironmentWindows } from './venvDeleteUtils'; diff --git a/src/test/common/configSettings.test.ts b/src/test/common/configSettings.test.ts index 8630835081e2..a8b4961f037c 100644 --- a/src/test/common/configSettings.test.ts +++ b/src/test/common/configSettings.test.ts @@ -4,7 +4,7 @@ import * as vscode from 'vscode'; import { SystemVariables } from '../../client/common/variables/systemVariables'; import { getExtensionSettings } from '../extensionSettings'; import { initialize } from './../initialize'; -import { isWindows } from '../../client/common/platform/platformService'; +import { isWindows } from '../../client/common/utils/platform'; const workspaceRoot = path.join(__dirname, '..', '..', '..', 'src', 'test'); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 7859b6d29e49..147803a72598 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -24,6 +24,7 @@ import { IServiceContainer } from '../../../client/ioc/types'; import { ITerminalAutoActivation } from '../../../client/terminals/types'; import { createPythonInterpreter } from '../../utils/interpreters'; import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis'; +import * as platform from '../../../client/common/utils/platform'; suite('Terminal Service', () => { let service: TerminalService; @@ -42,6 +43,7 @@ suite('Terminal Service', () => { let getConfigurationStub: sinon.SinonStub; let pythonConfig: TypeMoq.IMock; let editorConfig: TypeMoq.IMock; + let isWindowsStub: sinon.SinonStub; setup(() => { terminal = TypeMoq.Mock.ofType(); @@ -94,6 +96,7 @@ suite('Terminal Service', () => { mockServiceContainer.setup((c) => c.get(ITerminalActivator)).returns(() => terminalActivator.object); mockServiceContainer.setup((c) => c.get(ITerminalAutoActivation)).returns(() => terminalAutoActivator.object); getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration'); + isWindowsStub = sinon.stub(platform, 'isWindows'); pythonConfig = TypeMoq.Mock.ofType(); editorConfig = TypeMoq.Mock.ofType(); getConfigurationStub.callsFake((section: string) => { @@ -231,7 +234,8 @@ suite('Terminal Service', () => { terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.exactly(1)); }); - test('Ensure sendText is NOT called when Python shell integration and terminal shell integration are both enabled', async () => { + test('Ensure sendText is NOT called when Python shell integration and terminal shell integration are both enabled - Mac, Linux', async () => { + isWindowsStub.returns(false); pythonConfig .setup((p) => p.get('terminal.shellIntegration.enabled')) .returns(() => true) @@ -252,6 +256,28 @@ suite('Terminal Service', () => { terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.never()); }); + test('Ensure sendText IS called even when Python shell integration and terminal shell integration are both enabled - Window', async () => { + isWindowsStub.returns(true); + pythonConfig + .setup((p) => p.get('terminal.shellIntegration.enabled')) + .returns(() => true) + .verifiable(TypeMoq.Times.once()); + + terminalHelper + .setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => Promise.resolve(undefined)); + service = new TerminalService(mockServiceContainer.object); + const textToSend = 'Some Text'; + terminalHelper.setup((h) => h.identifyTerminalShell(TypeMoq.It.isAny())).returns(() => TerminalShellType.bash); + terminalManager.setup((t) => t.createTerminal(TypeMoq.It.isAny())).returns(() => terminal.object); + + service.ensureTerminal(); + service.executeCommand(textToSend, true); + + terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.exactly(1)); + terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.exactly(1)); + }); + test('Ensure terminal is not shown if `hideFromUser` option is set to `true`', async () => { terminalHelper .setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())) diff --git a/src/test/pythonEnvironments/nativeAPI.unit.test.ts b/src/test/pythonEnvironments/nativeAPI.unit.test.ts index 008d19b4738d..678a8fcfe2e3 100644 --- a/src/test/pythonEnvironments/nativeAPI.unit.test.ts +++ b/src/test/pythonEnvironments/nativeAPI.unit.test.ts @@ -13,9 +13,8 @@ import { NativeEnvManagerInfo, NativePythonFinder, } from '../../client/pythonEnvironments/base/locators/common/nativePythonFinder'; -import { Architecture } from '../../client/common/utils/platform'; +import { Architecture, isWindows } from '../../client/common/utils/platform'; import { PythonEnvInfo, PythonEnvKind, PythonEnvType } from '../../client/pythonEnvironments/base/info'; -import { isWindows } from '../../client/common/platform/platformService'; import { NativePythonEnvironmentKind } from '../../client/pythonEnvironments/base/locators/common/nativePythonUtils'; import * as condaApi from '../../client/pythonEnvironments/common/environmentManagers/conda'; import * as pyenvApi from '../../client/pythonEnvironments/common/environmentManagers/pyenv'; diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 9da8ec9a3fd3..a0919752cefd 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -7,7 +7,8 @@ import * as TypeMoq from 'typemoq'; import { Disposable, Memento } from 'vscode'; import { FileSystem } from '../client/common/platform/fileSystem'; import { PathUtils } from '../client/common/platform/pathUtils'; -import { PlatformService, isWindows } from '../client/common/platform/platformService'; +import { PlatformService } from '../client/common/platform/platformService'; +import { isWindows } from '../client/common/utils/platform'; import { RegistryImplementation } from '../client/common/platform/registry'; import { registerTypes as platformRegisterTypes } from '../client/common/platform/serviceRegistry'; import { IFileSystem, IPlatformService, IRegistry } from '../client/common/platform/types';