From e7803bebc4d30928dfebcd8dc1b516a028e61af9 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 13 Jun 2023 10:44:21 -0700 Subject: [PATCH] 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;