diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 45ce9afac47e..d3a9652acb1f 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -20,6 +20,7 @@ import { TerminalShellType, } from './types'; import { traceVerbose } from '../../logging'; +import { getConfiguration } from '../vscodeApis/workspaceApis'; @injectable() export class TerminalService implements ITerminalService, Disposable { @@ -64,7 +65,7 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(true); } - await this.executeCommand(text); + await this.executeCommand(text, false); } /** @deprecated */ public async sendText(text: string): Promise { @@ -74,7 +75,10 @@ export class TerminalService implements ITerminalService, Disposable { } this.terminal!.sendText(text); } - public async executeCommand(commandLine: string): Promise { + public async executeCommand( + commandLine: string, + isPythonShell: boolean, + ): Promise { const terminal = this.terminal!; if (!this.options?.hideFromUser) { terminal.show(true); @@ -98,7 +102,13 @@ export class TerminalService implements ITerminalService, Disposable { await promise; } - if (terminal.shellIntegration) { + const config = getConfiguration('python'); + const pythonrcSetting = config.get('terminal.shellIntegration.enabled'); + if (isPythonShell && !pythonrcSetting) { + // If user has explicitly disabled SI for Python, use sendText for inside Terminal REPL. + terminal.sendText(commandLine); + return undefined; + } else if (terminal.shellIntegration) { const execution = terminal.shellIntegration.executeCommand(commandLine); traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`); return execution; diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts index 60f8ed7a6847..0b46a86ee51e 100644 --- a/src/client/common/terminal/syncTerminalService.ts +++ b/src/client/common/terminal/syncTerminalService.ts @@ -145,8 +145,8 @@ export class SynchronousTerminalService implements ITerminalService, Disposable public sendText(text: string): Promise { return this.terminalService.sendText(text); } - public executeCommand(commandLine: string): Promise { - return this.terminalService.executeCommand(commandLine); + public executeCommand(commandLine: string, isPythonShell: boolean): Promise { + return this.terminalService.executeCommand(commandLine, isPythonShell); } public show(preserveFocus?: boolean | undefined): Promise { return this.terminalService.show(preserveFocus); diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index db2b7f80e4b1..3e54458a57fd 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -54,7 +54,7 @@ export interface ITerminalService extends IDisposable { ): Promise; /** @deprecated */ sendText(text: string): Promise; - executeCommand(commandLine: string): Promise; + executeCommand(commandLine: string, isPythonShell: boolean): Promise; show(preserveFocus?: boolean): Promise; } diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index 3cba6141763b..ea444af4d89e 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -59,7 +59,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource); } } else { - await this.getTerminalService(resource).executeCommand(code); + await this.getTerminalService(resource).executeCommand(code, true); } } diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index f0754948a233..7859b6d29e49 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import * as path from 'path'; +import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; import { Disposable, @@ -22,6 +23,7 @@ import { IDisposableRegistry } from '../../../client/common/types'; 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'; suite('Terminal Service', () => { let service: TerminalService; @@ -37,6 +39,9 @@ suite('Terminal Service', () => { let terminalShellIntegration: TypeMoq.IMock; let onDidEndTerminalShellExecutionEmitter: EventEmitter; let event: TerminalShellExecutionEndEvent; + let getConfigurationStub: sinon.SinonStub; + let pythonConfig: TypeMoq.IMock; + let editorConfig: TypeMoq.IMock; setup(() => { terminal = TypeMoq.Mock.ofType(); @@ -88,12 +93,22 @@ suite('Terminal Service', () => { mockServiceContainer.setup((c) => c.get(IWorkspaceService)).returns(() => workspaceService.object); mockServiceContainer.setup((c) => c.get(ITerminalActivator)).returns(() => terminalActivator.object); mockServiceContainer.setup((c) => c.get(ITerminalAutoActivation)).returns(() => terminalAutoActivator.object); + getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration'); + pythonConfig = TypeMoq.Mock.ofType(); + editorConfig = TypeMoq.Mock.ofType(); + getConfigurationStub.callsFake((section: string) => { + if (section === 'python') { + return pythonConfig.object; + } + return editorConfig.object; + }); }); teardown(() => { if (service) { service.dispose(); } disposables.filter((item) => !!item).forEach((item) => item.dispose()); + sinon.restore(); }); test('Ensure terminal is disposed', async () => { @@ -103,6 +118,7 @@ suite('Terminal Service', () => { const os: string = 'windows'; service = new TerminalService(mockServiceContainer.object); const shellPath = 'powershell.exe'; + // TODO: switch over legacy Terminal code to use workspace getConfiguration from workspaceApis instead of directly from vscode.workspace workspaceService .setup((w) => w.getConfiguration(TypeMoq.It.isValue('terminal.integrated.shell'))) .returns(() => { @@ -110,6 +126,7 @@ suite('Terminal Service', () => { workspaceConfig.setup((c) => c.get(os)).returns(() => shellPath); return workspaceConfig.object; }); + pythonConfig.setup((p) => p.get('terminal.shellIntegration.enabled')).returns(() => false); platformService.setup((p) => p.isWindows).returns(() => os === 'windows'); platformService.setup((p) => p.isLinux).returns(() => os === 'linux'); @@ -134,6 +151,7 @@ suite('Terminal Service', () => { }); test('Ensure command is sent to terminal and it is shown', async () => { + pythonConfig.setup((p) => p.get('terminal.shellIntegration.enabled')).returns(() => false); terminalHelper .setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(undefined)); @@ -171,6 +189,69 @@ suite('Terminal Service', () => { terminal.verify((t) => t.sendText(TypeMoq.It.isValue(textToSend)), TypeMoq.Times.exactly(1)); }); + test('Ensure sendText is used when Python shell integration is disabled', async () => { + pythonConfig + .setup((p) => p.get('terminal.shellIntegration.enabled')) + .returns(() => false) + .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 sendText is called when terminal.shellIntegration enabled but Python shell integration disabled', async () => { + pythonConfig + .setup((p) => p.get('terminal.shellIntegration.enabled')) + .returns(() => false) + .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 sendText is NOT called when Python shell integration and terminal shell integration are both enabled', async () => { + 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.never()); + }); + 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/terminals/codeExecution/terminalCodeExec.unit.test.ts b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts index 4b5537f515d2..b5bcecd971ea 100644 --- a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts +++ b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts @@ -643,10 +643,10 @@ suite('Terminal - Code Execution', () => { terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs); await executor.execute('cmd1'); - terminalService.verify(async (t) => t.executeCommand('cmd1'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd1', true), TypeMoq.Times.once()); await executor.execute('cmd2'); - terminalService.verify(async (t) => t.executeCommand('cmd2'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd2', true), TypeMoq.Times.once()); }); test('Ensure code is sent to the same terminal for a particular resource', async () => { @@ -668,10 +668,10 @@ suite('Terminal - Code Execution', () => { terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs); await executor.execute('cmd1', resource); - terminalService.verify(async (t) => t.executeCommand('cmd1'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd1', true), TypeMoq.Times.once()); await executor.execute('cmd2', resource); - terminalService.verify(async (t) => t.executeCommand('cmd2'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd2', true), TypeMoq.Times.once()); }); }); });