diff --git a/package.json b/package.json index 1401e0d1d645..8644028bcfe4 100644 --- a/package.json +++ b/package.json @@ -306,12 +306,6 @@ "icon": "$(play)", "title": "%python.command.python.execInTerminalIcon.title%" }, - { - "category": "Python", - "command": "python.execInDedicatedTerminal", - "icon": "$(play)", - "title": "%python.command.python.execInDedicatedTerminal.title%" - }, { "category": "Python", "command": "python.debugInTerminal", @@ -1642,13 +1636,6 @@ "title": "%python.command.python.execInTerminalIcon.title%", "when": "false" }, - { - "category": "Python", - "command": "python.execInDedicatedTerminal", - "icon": "$(play)", - "title": "%python.command.python.execInDedicatedTerminal.title%", - "when": "false" - }, { "category": "Python", "command": "python.debugInTerminal", @@ -1807,12 +1794,6 @@ "title": "%python.command.python.execInTerminalIcon.title%", "when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported" }, - { - "command": "python.execInDedicatedTerminal", - "group": "navigation@0", - "title": "%python.command.python.execInDedicatedTerminal.title%", - "when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported" - }, { "command": "python.debugInTerminal", "group": "navigation@1", diff --git a/package.nls.json b/package.nls.json index 71c4b5ee42ba..5d59e57a03a8 100644 --- a/package.nls.json +++ b/package.nls.json @@ -7,7 +7,6 @@ "python.command.python.execInTerminal.title": "Run Python File in Terminal", "python.command.python.debugInTerminal.title": "Debug Python File", "python.command.python.execInTerminalIcon.title": "Run Python File", - "python.command.python.execInDedicatedTerminal.title": "Run Python File in Dedicated Terminal", "python.command.python.setInterpreter.title": "Select Interpreter", "python.command.python.clearWorkspaceInterpreter.title": "Clear Workspace Interpreter Setting", "python.command.python.viewOutput.title": "Show Output", diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index bea0ef9e235c..b285667aaa6a 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -44,7 +44,6 @@ export namespace Commands { export const Enable_SourceMap_Support = 'python.enableSourceMapSupport'; export const Exec_In_Terminal = 'python.execInTerminal'; export const Exec_In_Terminal_Icon = 'python.execInTerminal-icon'; - export const Exec_In_Separate_Terminal = 'python.execInDedicatedTerminal'; export const Exec_Selection_In_Django_Shell = 'python.execSelectionInDjangoShell'; export const Exec_Selection_In_Terminal = 'python.execSelectionInTerminal'; export const GetSelectedInterpreterPath = 'python.interpreterPath'; diff --git a/src/client/common/terminal/factory.ts b/src/client/common/terminal/factory.ts index 39cc88c4b024..3855cb6cee3c 100644 --- a/src/client/common/terminal/factory.ts +++ b/src/client/common/terminal/factory.ts @@ -3,7 +3,6 @@ import { inject, injectable } from 'inversify'; import { Uri } from 'vscode'; -import * as path from 'path'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; import { PythonEnvironment } from '../../pythonEnvironments/info'; @@ -24,17 +23,13 @@ export class TerminalServiceFactory implements ITerminalServiceFactory { ) { this.terminalServices = new Map(); } - public getTerminalService(options: TerminalCreationOptions & { newTerminalPerFile?: boolean }): ITerminalService { + public getTerminalService(options: TerminalCreationOptions): ITerminalService { const resource = options?.resource; const title = options?.title; - let terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python'; + const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python'; const interpreter = options?.interpreter; - const id = this.getTerminalId(terminalTitle, resource, interpreter, options.newTerminalPerFile); + const id = this.getTerminalId(terminalTitle, resource, interpreter); if (!this.terminalServices.has(id)) { - if (resource && options.newTerminalPerFile) { - terminalTitle = `${terminalTitle}: ${path.basename(resource.fsPath).replace('.py', '')}`; - } - options.title = terminalTitle; const terminalService = new TerminalService(this.serviceContainer, options); this.terminalServices.set(id, terminalService); } @@ -51,19 +46,13 @@ export class TerminalServiceFactory implements ITerminalServiceFactory { title = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python'; return new TerminalService(this.serviceContainer, { resource, title }); } - private getTerminalId( - title: string, - resource?: Uri, - interpreter?: PythonEnvironment, - newTerminalPerFile?: boolean, - ): string { + private getTerminalId(title: string, resource?: Uri, interpreter?: PythonEnvironment): string { if (!resource && !interpreter) { return title; } const workspaceFolder = this.serviceContainer .get(IWorkspaceService) .getWorkspaceFolder(resource || undefined); - const fileId = resource && newTerminalPerFile ? resource.fsPath : ''; - return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}:${fileId}`; + return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}`; } } diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index 303188682378..880bf0dd72fb 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -97,7 +97,7 @@ export interface ITerminalServiceFactory { * @returns {ITerminalService} * @memberof ITerminalServiceFactory */ - getTerminalService(options: TerminalCreationOptions & { newTerminalPerFile?: boolean }): ITerminalService; + getTerminalService(options: TerminalCreationOptions): ITerminalService; createTerminalService(resource?: Uri, title?: string): ITerminalService; } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 20df3a21eb37..7881ada5653c 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -827,12 +827,6 @@ export interface IEventNamePropertyMapping { * @type {('command' | 'icon')} */ trigger?: 'command' | 'icon'; - /** - * Whether user chose to execute this Python file in a separate terminal or not. - * - * @type {boolean} - */ - newTerminalPerFile?: boolean; }; /** * Telemetry Event sent when user executes code against Django Shell. diff --git a/src/client/terminals/codeExecution/codeExecutionManager.ts b/src/client/terminals/codeExecution/codeExecutionManager.ts index 9f1ba6e90d90..ed671f2846a2 100644 --- a/src/client/terminals/codeExecution/codeExecutionManager.ts +++ b/src/client/terminals/codeExecution/codeExecutionManager.ts @@ -36,31 +36,25 @@ export class CodeExecutionManager implements ICodeExecutionManager { } public registerCommands() { - [Commands.Exec_In_Terminal, Commands.Exec_In_Terminal_Icon, Commands.Exec_In_Separate_Terminal].forEach( - (cmd) => { - this.disposableRegistry.push( - this.commandManager.registerCommand(cmd as any, async (file: Resource) => { - const interpreterService = this.serviceContainer.get(IInterpreterService); - const interpreter = await interpreterService.getActiveInterpreter(file); - if (!interpreter) { - this.commandManager - .executeCommand(Commands.TriggerEnvironmentSelection, file) - .then(noop, noop); - return; - } - const trigger = cmd === Commands.Exec_In_Terminal ? 'command' : 'icon'; - await this.executeFileInTerminal(file, trigger, { - newTerminalPerFile: cmd === Commands.Exec_In_Separate_Terminal, + [Commands.Exec_In_Terminal, Commands.Exec_In_Terminal_Icon].forEach((cmd) => { + this.disposableRegistry.push( + this.commandManager.registerCommand(cmd as any, async (file: Resource) => { + const interpreterService = this.serviceContainer.get(IInterpreterService); + const interpreter = await interpreterService.getActiveInterpreter(file); + if (!interpreter) { + this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop); + return; + } + const trigger = cmd === Commands.Exec_In_Terminal ? 'command' : 'icon'; + await this.executeFileInTerminal(file, trigger) + .then(() => { + if (this.shouldTerminalFocusOnStart(file)) + this.commandManager.executeCommand('workbench.action.terminal.focus'); }) - .then(() => { - if (this.shouldTerminalFocusOnStart(file)) - this.commandManager.executeCommand('workbench.action.terminal.focus'); - }) - .catch((ex) => traceError('Failed to execute file in terminal', ex)); - }), - ); - }, - ); + .catch((ex) => traceError('Failed to execute file in terminal', ex)); + }), + ); + }); this.disposableRegistry.push( this.commandManager.registerCommand(Commands.Exec_Selection_In_Terminal as any, async (file: Resource) => { const interpreterService = this.serviceContainer.get(IInterpreterService); @@ -93,16 +87,8 @@ export class CodeExecutionManager implements ICodeExecutionManager { ), ); } - private async executeFileInTerminal( - file: Resource, - trigger: 'command' | 'icon', - options?: { newTerminalPerFile: boolean }, - ): Promise { - sendTelemetryEvent(EventName.EXECUTION_CODE, undefined, { - scope: 'file', - trigger, - newTerminalPerFile: options?.newTerminalPerFile, - }); + private async executeFileInTerminal(file: Resource, trigger: 'command' | 'icon') { + sendTelemetryEvent(EventName.EXECUTION_CODE, undefined, { scope: 'file', trigger }); const codeExecutionHelper = this.serviceContainer.get(ICodeExecutionHelper); file = file instanceof Uri ? file : undefined; let fileToExecute = file ? file : await codeExecutionHelper.getFileToExecute(); @@ -124,7 +110,7 @@ export class CodeExecutionManager implements ICodeExecutionManager { } const executionService = this.serviceContainer.get(ICodeExecutionService, 'standard'); - await executionService.executeFile(fileToExecute, options); + await executionService.executeFile(fileToExecute); } @captureTelemetry(EventName.EXECUTION_CODE, { scope: 'selection' }, false) diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index ca7488530f75..9261483b45e1 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -19,6 +19,7 @@ import { ICodeExecutionService } from '../../terminals/types'; export class TerminalCodeExecutionProvider implements ICodeExecutionService { private hasRanOutsideCurrentDrive = false; protected terminalTitle!: string; + private _terminalService!: ITerminalService; private replActive?: Promise; constructor( @inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory, @@ -29,13 +30,13 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { @inject(IInterpreterService) protected readonly interpreterService: IInterpreterService, ) {} - public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) { + public async executeFile(file: Uri) { await this.setCwdForFileExecution(file); const { command, args } = await this.getExecuteFileArgs(file, [ file.fsPath.fileToCommandArgumentForPythonExt(), ]); - await this.getTerminalService(file, options).sendCommand(command, args); + await this.getTerminalService(file).sendCommand(command, args); } public async execute(code: string, resource?: Uri): Promise { @@ -47,23 +48,17 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { await this.getTerminalService(resource).sendText(code); } public async initializeRepl(resource?: Uri) { - const terminalService = this.getTerminalService(resource); if (this.replActive && (await this.replActive)) { - await terminalService.show(); + await this._terminalService.show(); return; } this.replActive = new Promise(async (resolve) => { const replCommandArgs = await this.getExecutableInfo(resource); - terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); + await this.getTerminalService(resource).sendCommand(replCommandArgs.command, replCommandArgs.args); // Give python repl time to start before we start sending text. setTimeout(() => resolve(true), 1000); }); - this.disposables.push( - terminalService.onDidCloseTerminal(() => { - this.replActive = undefined; - }), - ); await this.replActive; } @@ -81,12 +76,19 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { public async getExecuteFileArgs(resource?: Uri, executeArgs: string[] = []): Promise { return this.getExecutableInfo(resource, executeArgs); } - private getTerminalService(resource?: Uri, options?: { newTerminalPerFile: boolean }): ITerminalService { - return this.terminalServiceFactory.getTerminalService({ - resource, - title: this.terminalTitle, - newTerminalPerFile: options?.newTerminalPerFile, - }); + private getTerminalService(resource?: Uri): ITerminalService { + if (!this._terminalService) { + this._terminalService = this.terminalServiceFactory.getTerminalService({ + resource, + title: this.terminalTitle, + }); + this.disposables.push( + this._terminalService.onDidCloseTerminal(() => { + this.replActive = undefined; + }), + ); + } + return this._terminalService; } private async setCwdForFileExecution(file: Uri) { const pythonSettings = this.configurationService.getSettings(file); diff --git a/src/client/terminals/types.ts b/src/client/terminals/types.ts index 47ac16d9e08b..cf31f4ef1dd0 100644 --- a/src/client/terminals/types.ts +++ b/src/client/terminals/types.ts @@ -8,7 +8,7 @@ export const ICodeExecutionService = Symbol('ICodeExecutionService'); export interface ICodeExecutionService { execute(code: string, resource?: Uri): Promise; - executeFile(file: Uri, options?: { newTerminalPerFile: boolean }): Promise; + executeFile(file: Uri): Promise; initializeRepl(resource?: Uri): Promise; } diff --git a/src/test/common/terminals/factory.unit.test.ts b/src/test/common/terminals/factory.unit.test.ts index 5ad2da8e793a..ef6b7d8f5b0f 100644 --- a/src/test/common/terminals/factory.unit.test.ts +++ b/src/test/common/terminals/factory.unit.test.ts @@ -105,7 +105,7 @@ suite('Terminal Service Factory', () => { expect(notSameAsThirdInstance).to.not.equal(true, 'Instances are the same'); }); - test('Ensure same terminal is returned when using different resources from the same workspace', () => { + test('Ensure same terminal is returned when using resources from the same workspace', () => { const file1A = Uri.file('1a'); const file2A = Uri.file('2a'); const fileB = Uri.file('b'); @@ -140,49 +140,4 @@ suite('Terminal Service Factory', () => { 'Instances should be different for different workspaces', ); }); - - test('When `newTerminalPerFile` is true, ensure different terminal is returned when using different resources from the same workspace', () => { - const file1A = Uri.file('1a'); - const file2A = Uri.file('2a'); - const fileB = Uri.file('b'); - const workspaceUriA = Uri.file('A'); - const workspaceUriB = Uri.file('B'); - const workspaceFolderA = TypeMoq.Mock.ofType(); - workspaceFolderA.setup((w) => w.uri).returns(() => workspaceUriA); - const workspaceFolderB = TypeMoq.Mock.ofType(); - workspaceFolderB.setup((w) => w.uri).returns(() => workspaceUriB); - - workspaceService - .setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(file1A))) - .returns(() => workspaceFolderA.object); - workspaceService - .setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(file2A))) - .returns(() => workspaceFolderA.object); - workspaceService - .setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(fileB))) - .returns(() => workspaceFolderB.object); - - const terminalForFile1A = factory.getTerminalService({ - resource: file1A, - newTerminalPerFile: true, - }) as SynchronousTerminalService; - const terminalForFile2A = factory.getTerminalService({ - resource: file2A, - newTerminalPerFile: true, - }) as SynchronousTerminalService; - const terminalForFileB = factory.getTerminalService({ - resource: fileB, - newTerminalPerFile: true, - }) as SynchronousTerminalService; - - const terminalsAreSameForWorkspaceA = terminalForFile1A.terminalService === terminalForFile2A.terminalService; - expect(terminalsAreSameForWorkspaceA).to.equal(false, 'Instances are the same for Workspace A'); - - const terminalsForWorkspaceABAreDifferent = - terminalForFile1A.terminalService === terminalForFileB.terminalService; - expect(terminalsForWorkspaceABAreDifferent).to.equal( - false, - 'Instances should be different for different workspaces', - ); - }); }); diff --git a/src/test/terminals/codeExecution/codeExecutionManager.unit.test.ts b/src/test/terminals/codeExecution/codeExecutionManager.unit.test.ts index 30f95c94d217..3676834873a0 100644 --- a/src/test/terminals/codeExecution/codeExecutionManager.unit.test.ts +++ b/src/test/terminals/codeExecution/codeExecutionManager.unit.test.ts @@ -77,15 +77,12 @@ suite('Terminal - Code Execution Manager', () => { executionManager.registerCommands(); const sorted = registered.sort(); - expect(sorted).to.deep.equal( - [ - Commands.Exec_In_Separate_Terminal, - Commands.Exec_In_Terminal, - Commands.Exec_In_Terminal_Icon, - Commands.Exec_Selection_In_Django_Shell, - Commands.Exec_Selection_In_Terminal, - ].sort(), - ); + expect(sorted).to.deep.equal([ + Commands.Exec_In_Terminal, + Commands.Exec_In_Terminal_Icon, + Commands.Exec_Selection_In_Django_Shell, + Commands.Exec_Selection_In_Terminal, + ]); }); test('Ensure executeFileInterTerminal will do nothing if no file is avialble', async () => { @@ -138,10 +135,7 @@ suite('Terminal - Code Execution Manager', () => { const fileToExecute = Uri.file('x'); await commandHandler!(fileToExecute); helper.verify(async (h) => h.getFileToExecute(), TypeMoq.Times.never()); - executionService.verify( - async (e) => e.executeFile(TypeMoq.It.isValue(fileToExecute), TypeMoq.It.isAny()), - TypeMoq.Times.once(), - ); + executionService.verify(async (e) => e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once()); }); test('Ensure executeFileInterTerminal will use active file', async () => { @@ -170,10 +164,7 @@ suite('Terminal - Code Execution Manager', () => { .returns(() => executionService.object); await commandHandler!(fileToExecute); - executionService.verify( - async (e) => e.executeFile(TypeMoq.It.isValue(fileToExecute), TypeMoq.It.isAny()), - TypeMoq.Times.once(), - ); + executionService.verify(async (e) => e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once()); }); async function testExecutionOfSelectionWithoutAnyActiveDocument(commandId: string, executionSericeId: string) {