From b06bc5c5e4f008388305a8d75eeffe869bc4fb3b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 13 Jun 2023 10:41:43 -0700 Subject: [PATCH 1/3] Revert "Change name of command to run Python files in separate terminals (#21229)" This reverts commit 0c4fa40d28bcdd2848c36499be4be61a1a90a4e5. --- package.json | 12 ++++++------ package.nls.json | 2 +- src/client/common/constants.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 1401e0d1d645..a6888f8706b8 100644 --- a/package.json +++ b/package.json @@ -308,9 +308,9 @@ }, { "category": "Python", - "command": "python.execInDedicatedTerminal", + "command": "python.execInNewTerminal", "icon": "$(play)", - "title": "%python.command.python.execInDedicatedTerminal.title%" + "title": "%python.command.python.execInNewTerminal.title%" }, { "category": "Python", @@ -1644,9 +1644,9 @@ }, { "category": "Python", - "command": "python.execInDedicatedTerminal", + "command": "python.execInNewTerminal", "icon": "$(play)", - "title": "%python.command.python.execInDedicatedTerminal.title%", + "title": "%python.command.python.execInNewTerminal.title%", "when": "false" }, { @@ -1808,9 +1808,9 @@ "when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported" }, { - "command": "python.execInDedicatedTerminal", + "command": "python.execInNewTerminal", "group": "navigation@0", - "title": "%python.command.python.execInDedicatedTerminal.title%", + "title": "%python.command.python.execInNewTerminal.title%", "when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported" }, { diff --git a/package.nls.json b/package.nls.json index 71c4b5ee42ba..ff5ecdbe23dc 100644 --- a/package.nls.json +++ b/package.nls.json @@ -7,7 +7,7 @@ "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.execInNewTerminal.title": "Run Python File in Separate 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..aae792ec2e39 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -44,7 +44,7 @@ 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_In_Separate_Terminal = 'python.execInNewTerminal'; export const Exec_Selection_In_Django_Shell = 'python.execSelectionInDjangoShell'; export const Exec_Selection_In_Terminal = 'python.execSelectionInTerminal'; export const GetSelectedInterpreterPath = 'python.interpreterPath'; From 6601c8da2edd891d6c79f5e86fd4fb00bfbd1acc Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 13 Jun 2023 10:43:43 -0700 Subject: [PATCH 2/3] Revert "Added option to run multiple Python files in separate terminals (#21223)" This reverts commit 15338189257cdf264a9a00966c8909d6a06123e8. --- package.json | 19 ------- package.nls.json | 1 - src/client/common/constants.ts | 1 - src/client/common/terminal/factory.ts | 16 ++---- src/client/common/terminal/types.ts | 2 +- src/client/telemetry/index.ts | 6 -- .../codeExecution/codeExecutionManager.ts | 56 +++++++------------ .../codeExecution/terminalCodeExecution.ts | 19 ++++--- src/client/terminals/types.ts | 2 +- .../common/terminals/factory.unit.test.ts | 47 +--------------- .../codeExecutionManager.unit.test.ts | 25 +++------ 11 files changed, 47 insertions(+), 147 deletions(-) diff --git a/package.json b/package.json index a6888f8706b8..8644028bcfe4 100644 --- a/package.json +++ b/package.json @@ -306,12 +306,6 @@ "icon": "$(play)", "title": "%python.command.python.execInTerminalIcon.title%" }, - { - "category": "Python", - "command": "python.execInNewTerminal", - "icon": "$(play)", - "title": "%python.command.python.execInNewTerminal.title%" - }, { "category": "Python", "command": "python.debugInTerminal", @@ -1642,13 +1636,6 @@ "title": "%python.command.python.execInTerminalIcon.title%", "when": "false" }, - { - "category": "Python", - "command": "python.execInNewTerminal", - "icon": "$(play)", - "title": "%python.command.python.execInNewTerminal.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.execInNewTerminal", - "group": "navigation@0", - "title": "%python.command.python.execInNewTerminal.title%", - "when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported" - }, { "command": "python.debugInTerminal", "group": "navigation@1", diff --git a/package.nls.json b/package.nls.json index ff5ecdbe23dc..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.execInNewTerminal.title": "Run Python File in Separate 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 aae792ec2e39..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.execInNewTerminal'; 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..3cbe123b5629 100644 --- a/src/client/common/terminal/factory.ts +++ b/src/client/common/terminal/factory.ts @@ -24,14 +24,14 @@ 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 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) { + if (this.terminalServices.size >= 1 && resource) { terminalTitle = `${terminalTitle}: ${path.basename(resource.fsPath).replace('.py', '')}`; } options.title = terminalTitle; @@ -51,19 +51,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}:${resource?.fsPath || ''}`; } } 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..b604d062e81e 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -19,7 +19,7 @@ import { ICodeExecutionService } from '../../terminals/types'; export class TerminalCodeExecutionProvider implements ICodeExecutionService { private hasRanOutsideCurrentDrive = false; protected terminalTitle!: string; - private replActive?: Promise; + private replActive = new Map>(); constructor( @inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory, @inject(IConfigurationService) protected readonly configurationService: IConfigurationService, @@ -29,13 +29,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 { @@ -48,24 +48,26 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { } public async initializeRepl(resource?: Uri) { const terminalService = this.getTerminalService(resource); - if (this.replActive && (await this.replActive)) { + let replActive = this.replActive.get(resource?.fsPath || ''); + if (replActive && (await replActive)) { await terminalService.show(); return; } - this.replActive = new Promise(async (resolve) => { + replActive = new Promise(async (resolve) => { const replCommandArgs = await this.getExecutableInfo(resource); terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); // Give python repl time to start before we start sending text. setTimeout(() => resolve(true), 1000); }); + this.replActive.set(resource?.fsPath || '', replActive); this.disposables.push( terminalService.onDidCloseTerminal(() => { - this.replActive = undefined; + this.replActive.delete(resource?.fsPath || ''); }), ); - await this.replActive; + await replActive; } public async getExecutableInfo(resource?: Uri, args: string[] = []): Promise { @@ -81,11 +83,10 @@ 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 { + private getTerminalService(resource?: Uri): ITerminalService { return this.terminalServiceFactory.getTerminalService({ resource, title: this.terminalTitle, - newTerminalPerFile: options?.newTerminalPerFile, }); } private async setCwdForFileExecution(file: Uri) { 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..f01d5a85fbb5 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 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'); @@ -130,51 +130,6 @@ suite('Terminal Service Factory', () => { const terminalForFile2A = factory.getTerminalService({ resource: file2A }) as SynchronousTerminalService; const terminalForFileB = factory.getTerminalService({ resource: fileB }) as SynchronousTerminalService; - const terminalsAreSameForWorkspaceA = terminalForFile1A.terminalService === terminalForFile2A.terminalService; - expect(terminalsAreSameForWorkspaceA).to.equal(true, 'Instances are not the same for Workspace A'); - - const terminalsForWorkspaceABAreDifferent = - terminalForFile1A.terminalService === terminalForFileB.terminalService; - expect(terminalsForWorkspaceABAreDifferent).to.equal( - false, - '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'); 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) { From e7803bebc4d30928dfebcd8dc1b516a028e61af9 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 13 Jun 2023 10:44:21 -0700 Subject: [PATCH 3/3] Revert "Open separate Python terminals when running different Python files (#21202)" This reverts commit 17daae4208001c894229bbe794822e5f3d2fcb5e. --- src/client/common/terminal/factory.ts | 9 +---- .../codeExecution/terminalCodeExecution.ts | 37 ++++++++++--------- .../common/terminals/factory.unit.test.ts | 4 +- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/client/common/terminal/factory.ts b/src/client/common/terminal/factory.ts index 3cbe123b5629..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'; @@ -27,14 +26,10 @@ export class TerminalServiceFactory implements ITerminalServiceFactory { 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); if (!this.terminalServices.has(id)) { - if (this.terminalServices.size >= 1 && resource) { - terminalTitle = `${terminalTitle}: ${path.basename(resource.fsPath).replace('.py', '')}`; - } - options.title = terminalTitle; const terminalService = new TerminalService(this.serviceContainer, options); this.terminalServices.set(id, terminalService); } @@ -58,6 +53,6 @@ export class TerminalServiceFactory implements ITerminalServiceFactory { const workspaceFolder = this.serviceContainer .get(IWorkspaceService) .getWorkspaceFolder(resource || undefined); - return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}:${resource?.fsPath || ''}`; + return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}`; } } diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index b604d062e81e..9261483b45e1 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -19,7 +19,8 @@ import { ICodeExecutionService } from '../../terminals/types'; export class TerminalCodeExecutionProvider implements ICodeExecutionService { private hasRanOutsideCurrentDrive = false; protected terminalTitle!: string; - private replActive = new Map>(); + private _terminalService!: ITerminalService; + private replActive?: Promise; constructor( @inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory, @inject(IConfigurationService) protected readonly configurationService: IConfigurationService, @@ -47,27 +48,19 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { await this.getTerminalService(resource).sendText(code); } public async initializeRepl(resource?: Uri) { - const terminalService = this.getTerminalService(resource); - let replActive = this.replActive.get(resource?.fsPath || ''); - if (replActive && (await replActive)) { - await terminalService.show(); + if (this.replActive && (await this.replActive)) { + await this._terminalService.show(); return; } - replActive = new Promise(async (resolve) => { + 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.replActive.set(resource?.fsPath || '', replActive); - this.disposables.push( - terminalService.onDidCloseTerminal(() => { - this.replActive.delete(resource?.fsPath || ''); - }), - ); - await replActive; + await this.replActive; } public async getExecutableInfo(resource?: Uri, args: string[] = []): Promise { @@ -84,10 +77,18 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { return this.getExecutableInfo(resource, executeArgs); } private getTerminalService(resource?: Uri): ITerminalService { - return this.terminalServiceFactory.getTerminalService({ - resource, - title: this.terminalTitle, - }); + 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/test/common/terminals/factory.unit.test.ts b/src/test/common/terminals/factory.unit.test.ts index f01d5a85fbb5..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 different 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'); @@ -131,7 +131,7 @@ suite('Terminal Service Factory', () => { const terminalForFileB = factory.getTerminalService({ resource: fileB }) as SynchronousTerminalService; const terminalsAreSameForWorkspaceA = terminalForFile1A.terminalService === terminalForFile2A.terminalService; - expect(terminalsAreSameForWorkspaceA).to.equal(false, 'Instances are the same for Workspace A'); + expect(terminalsAreSameForWorkspaceA).to.equal(true, 'Instances are not the same for Workspace A'); const terminalsForWorkspaceABAreDifferent = terminalForFile1A.terminalService === terminalForFileB.terminalService;