Skip to content

Open separate Python terminals when running different Python files #21202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/client/common/terminal/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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';
Expand All @@ -26,10 +27,14 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
public getTerminalService(options: TerminalCreationOptions): ITerminalService {
const resource = options?.resource;
const title = options?.title;
const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
let 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);
}
Expand All @@ -53,6 +58,6 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
const workspaceFolder = this.serviceContainer
.get<IWorkspaceService>(IWorkspaceService)
.getWorkspaceFolder(resource || undefined);
return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}`;
return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}:${resource?.fsPath || ''}`;
}
}
37 changes: 18 additions & 19 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import { ICodeExecutionService } from '../../terminals/types';
export class TerminalCodeExecutionProvider implements ICodeExecutionService {
private hasRanOutsideCurrentDrive = false;
protected terminalTitle!: string;
private _terminalService!: ITerminalService;
private replActive?: Promise<boolean>;
private replActive = new Map<string, Promise<boolean>>();
constructor(
@inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory,
@inject(IConfigurationService) protected readonly configurationService: IConfigurationService,
Expand Down Expand Up @@ -48,19 +47,27 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
await this.getTerminalService(resource).sendText(code);
}
public async initializeRepl(resource?: Uri) {
if (this.replActive && (await this.replActive)) {
await this._terminalService.show();
const terminalService = this.getTerminalService(resource);
let replActive = this.replActive.get(resource?.fsPath || '');
if (replActive && (await replActive)) {
await terminalService.show();
return;
}
this.replActive = new Promise<boolean>(async (resolve) => {
replActive = new Promise<boolean>(async (resolve) => {
const replCommandArgs = await this.getExecutableInfo(resource);
await this.getTerminalService(resource).sendCommand(replCommandArgs.command, replCommandArgs.args);
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.delete(resource?.fsPath || '');
}),
);

await this.replActive;
await replActive;
}

public async getExecutableInfo(resource?: Uri, args: string[] = []): Promise<PythonExecInfo> {
Expand All @@ -77,18 +84,10 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
return this.getExecutableInfo(resource, executeArgs);
}
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;
return this.terminalServiceFactory.getTerminalService({
resource,
title: this.terminalTitle,
});
}
private async setCwdForFileExecution(file: Uri) {
const pythonSettings = this.configurationService.getSettings(file);
Expand Down
4 changes: 2 additions & 2 deletions src/test/common/terminals/factory.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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');
Expand All @@ -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(true, 'Instances are not the same for Workspace A');
expect(terminalsAreSameForWorkspaceA).to.equal(false, 'Instances are the same for Workspace A');

const terminalsForWorkspaceABAreDifferent =
terminalForFile1A.terminalService === terminalForFileB.terminalService;
Expand Down