Fix: Don’t use executeCommand on python sub-shell for windows. (#24542)

anthonykim1 · web-flow · commit 4153f7c233b3 · 2024-12-04T11:27:06.000+05:30
Related: #24418 Further resolves: #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)
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<boolean>('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
@@ -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
@@ -7,15 +7,14 @@ 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';
 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
@@ -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
@@ -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
@@ -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<void> {
     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
@@ -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
@@ -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
@@ -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<WorkspaceConfiguration>;
     let editorConfig: TypeMoq.IMock<WorkspaceConfiguration>;
+    let isWindowsStub: sinon.SinonStub;
 
     setup(() => {
         terminal = TypeMoq.Mock.ofType<VSCodeTerminal>();
@@ -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<WorkspaceConfiguration>();
         editorConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
         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
@@ -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
@@ -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';

Original file line number	Diff line number	Diff line change
`@@ -67,3 +67,7 @@ export function getUserHomeDir(): string \| undefined {`
`67`	`67`	`}`
`68`	`68`	`return getEnvironmentVariable('HOME') \|\| getEnvironmentVariable('HOMEPATH');`
`69`	`69`	`}`
	`70`	`+`
	`71`	`+export function isWindows(): boolean {`
	`72`	`+ return getOSType() === OSType.Windows;`
	`73`	`+}`