Skip to content

Commit 81ae205

Browse files
author
Kartik Raj
authored
Bring back feature to Run Python file in dedicated terminal (#21656)
Closes #21282 Closes #21420 Closes #21215 Reverts #21418
1 parent c144200 commit 81ae205

File tree

12 files changed

+190
-59
lines changed

12 files changed

+190
-59
lines changed

package.json

+19
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,12 @@
392392
"icon": "$(play)",
393393
"title": "%python.command.python.execInTerminalIcon.title%"
394394
},
395+
{
396+
"category": "Python",
397+
"command": "python.execInDedicatedTerminal",
398+
"icon": "$(play)",
399+
"title": "%python.command.python.execInDedicatedTerminal.title%"
400+
},
395401
{
396402
"category": "Python",
397403
"command": "python.debugInTerminal",
@@ -1818,6 +1824,13 @@
18181824
"title": "%python.command.python.execInTerminalIcon.title%",
18191825
"when": "false"
18201826
},
1827+
{
1828+
"category": "Python",
1829+
"command": "python.execInDedicatedTerminal",
1830+
"icon": "$(play)",
1831+
"title": "%python.command.python.execInDedicatedTerminal.title%",
1832+
"when": "false"
1833+
},
18211834
{
18221835
"category": "Python",
18231836
"command": "python.debugInTerminal",
@@ -1976,6 +1989,12 @@
19761989
"title": "%python.command.python.execInTerminalIcon.title%",
19771990
"when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported"
19781991
},
1992+
{
1993+
"command": "python.execInDedicatedTerminal",
1994+
"group": "navigation@0",
1995+
"title": "%python.command.python.execInDedicatedTerminal.title%",
1996+
"when": "resourceLangId == python && !isInDiffEditor && !virtualWorkspace && shellExecutionSupported"
1997+
},
19791998
{
19801999
"command": "python.debugInTerminal",
19812000
"group": "navigation@1",

package.nls.json

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"python.command.python.execInTerminal.title": "Run Python File in Terminal",
88
"python.command.python.debugInTerminal.title": "Debug Python File",
99
"python.command.python.execInTerminalIcon.title": "Run Python File",
10+
"python.command.python.execInDedicatedTerminal.title": "Run Python File in Dedicated Terminal",
1011
"python.command.python.setInterpreter.title": "Select Interpreter",
1112
"python.command.python.clearWorkspaceInterpreter.title": "Clear Workspace Interpreter Setting",
1213
"python.command.python.viewOutput.title": "Show Output",

src/client/common/constants.ts

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export namespace Commands {
4444
export const Enable_SourceMap_Support = 'python.enableSourceMapSupport';
4545
export const Exec_In_Terminal = 'python.execInTerminal';
4646
export const Exec_In_Terminal_Icon = 'python.execInTerminal-icon';
47+
export const Exec_In_Separate_Terminal = 'python.execInDedicatedTerminal';
4748
export const Exec_Selection_In_Django_Shell = 'python.execSelectionInDjangoShell';
4849
export const Exec_Selection_In_Terminal = 'python.execSelectionInTerminal';
4950
export const GetSelectedInterpreterPath = 'python.interpreterPath';

src/client/common/terminal/factory.ts

+16-5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { inject, injectable } from 'inversify';
55
import { Uri } from 'vscode';
6+
import * as path from 'path';
67
import { IInterpreterService } from '../../interpreter/contracts';
78
import { IServiceContainer } from '../../ioc/types';
89
import { PythonEnvironment } from '../../pythonEnvironments/info';
@@ -23,13 +24,17 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
2324
) {
2425
this.terminalServices = new Map<string, TerminalService>();
2526
}
26-
public getTerminalService(options: TerminalCreationOptions): ITerminalService {
27+
public getTerminalService(options: TerminalCreationOptions & { newTerminalPerFile?: boolean }): ITerminalService {
2728
const resource = options?.resource;
2829
const title = options?.title;
29-
const terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
30+
let terminalTitle = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
3031
const interpreter = options?.interpreter;
31-
const id = this.getTerminalId(terminalTitle, resource, interpreter);
32+
const id = this.getTerminalId(terminalTitle, resource, interpreter, options.newTerminalPerFile);
3233
if (!this.terminalServices.has(id)) {
34+
if (resource && options.newTerminalPerFile) {
35+
terminalTitle = `${terminalTitle}: ${path.basename(resource.fsPath).replace('.py', '')}`;
36+
}
37+
options.title = terminalTitle;
3338
const terminalService = new TerminalService(this.serviceContainer, options);
3439
this.terminalServices.set(id, terminalService);
3540
}
@@ -46,13 +51,19 @@ export class TerminalServiceFactory implements ITerminalServiceFactory {
4651
title = typeof title === 'string' && title.trim().length > 0 ? title.trim() : 'Python';
4752
return new TerminalService(this.serviceContainer, { resource, title });
4853
}
49-
private getTerminalId(title: string, resource?: Uri, interpreter?: PythonEnvironment): string {
54+
private getTerminalId(
55+
title: string,
56+
resource?: Uri,
57+
interpreter?: PythonEnvironment,
58+
newTerminalPerFile?: boolean,
59+
): string {
5060
if (!resource && !interpreter) {
5161
return title;
5262
}
5363
const workspaceFolder = this.serviceContainer
5464
.get<IWorkspaceService>(IWorkspaceService)
5565
.getWorkspaceFolder(resource || undefined);
56-
return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}`;
66+
const fileId = resource && newTerminalPerFile ? resource.fsPath : '';
67+
return `${title}:${workspaceFolder?.uri.fsPath || ''}:${interpreter?.path}:${fileId}`;
5768
}
5869
}

src/client/common/terminal/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export interface ITerminalServiceFactory {
9797
* @returns {ITerminalService}
9898
* @memberof ITerminalServiceFactory
9999
*/
100-
getTerminalService(options: TerminalCreationOptions): ITerminalService;
100+
getTerminalService(options: TerminalCreationOptions & { newTerminalPerFile?: boolean }): ITerminalService;
101101
createTerminalService(resource?: Uri, title?: string): ITerminalService;
102102
}
103103

src/client/telemetry/index.ts

+6
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,12 @@ export interface IEventNamePropertyMapping {
827827
* @type {('command' | 'icon')}
828828
*/
829829
trigger?: 'command' | 'icon';
830+
/**
831+
* Whether user chose to execute this Python file in a separate terminal or not.
832+
*
833+
* @type {boolean}
834+
*/
835+
newTerminalPerFile?: boolean;
830836
};
831837
/**
832838
* Telemetry Event sent when user executes code against Django Shell.

src/client/terminals/codeExecution/codeExecutionManager.ts

+35-21
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,31 @@ export class CodeExecutionManager implements ICodeExecutionManager {
3636
}
3737

3838
public registerCommands() {
39-
[Commands.Exec_In_Terminal, Commands.Exec_In_Terminal_Icon].forEach((cmd) => {
40-
this.disposableRegistry.push(
41-
this.commandManager.registerCommand(cmd as any, async (file: Resource) => {
42-
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
43-
const interpreter = await interpreterService.getActiveInterpreter(file);
44-
if (!interpreter) {
45-
this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop);
46-
return;
47-
}
48-
const trigger = cmd === Commands.Exec_In_Terminal ? 'command' : 'icon';
49-
await this.executeFileInTerminal(file, trigger)
50-
.then(() => {
51-
if (this.shouldTerminalFocusOnStart(file))
52-
this.commandManager.executeCommand('workbench.action.terminal.focus');
39+
[Commands.Exec_In_Terminal, Commands.Exec_In_Terminal_Icon, Commands.Exec_In_Separate_Terminal].forEach(
40+
(cmd) => {
41+
this.disposableRegistry.push(
42+
this.commandManager.registerCommand(cmd as any, async (file: Resource) => {
43+
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
44+
const interpreter = await interpreterService.getActiveInterpreter(file);
45+
if (!interpreter) {
46+
this.commandManager
47+
.executeCommand(Commands.TriggerEnvironmentSelection, file)
48+
.then(noop, noop);
49+
return;
50+
}
51+
const trigger = cmd === Commands.Exec_In_Terminal ? 'command' : 'icon';
52+
await this.executeFileInTerminal(file, trigger, {
53+
newTerminalPerFile: cmd === Commands.Exec_In_Separate_Terminal,
5354
})
54-
.catch((ex) => traceError('Failed to execute file in terminal', ex));
55-
}),
56-
);
57-
});
55+
.then(() => {
56+
if (this.shouldTerminalFocusOnStart(file))
57+
this.commandManager.executeCommand('workbench.action.terminal.focus');
58+
})
59+
.catch((ex) => traceError('Failed to execute file in terminal', ex));
60+
}),
61+
);
62+
},
63+
);
5864
this.disposableRegistry.push(
5965
this.commandManager.registerCommand(Commands.Exec_Selection_In_Terminal as any, async (file: Resource) => {
6066
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
@@ -87,8 +93,16 @@ export class CodeExecutionManager implements ICodeExecutionManager {
8793
),
8894
);
8995
}
90-
private async executeFileInTerminal(file: Resource, trigger: 'command' | 'icon') {
91-
sendTelemetryEvent(EventName.EXECUTION_CODE, undefined, { scope: 'file', trigger });
96+
private async executeFileInTerminal(
97+
file: Resource,
98+
trigger: 'command' | 'icon',
99+
options?: { newTerminalPerFile: boolean },
100+
): Promise<void> {
101+
sendTelemetryEvent(EventName.EXECUTION_CODE, undefined, {
102+
scope: 'file',
103+
trigger,
104+
newTerminalPerFile: options?.newTerminalPerFile,
105+
});
92106
const codeExecutionHelper = this.serviceContainer.get<ICodeExecutionHelper>(ICodeExecutionHelper);
93107
file = file instanceof Uri ? file : undefined;
94108
let fileToExecute = file ? file : await codeExecutionHelper.getFileToExecute();
@@ -110,7 +124,7 @@ export class CodeExecutionManager implements ICodeExecutionManager {
110124
}
111125

112126
const executionService = this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService, 'standard');
113-
await executionService.executeFile(fileToExecute);
127+
await executionService.executeFile(fileToExecute, options);
114128
}
115129

116130
@captureTelemetry(EventName.EXECUTION_CODE, { scope: 'selection' }, false)

src/client/terminals/codeExecution/terminalCodeExecution.ts

+19-21
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { IWorkspaceService } from '../../common/application/types';
1010
import '../../common/extensions';
1111
import { IPlatformService } from '../../common/platform/types';
1212
import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
13-
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
13+
import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
1414
import { IInterpreterService } from '../../interpreter/contracts';
1515
import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
1616
import { ICodeExecutionService } from '../../terminals/types';
@@ -19,7 +19,6 @@ import { ICodeExecutionService } from '../../terminals/types';
1919
export class TerminalCodeExecutionProvider implements ICodeExecutionService {
2020
private hasRanOutsideCurrentDrive = false;
2121
protected terminalTitle!: string;
22-
private _terminalService!: ITerminalService;
2322
private replActive?: Promise<boolean>;
2423
constructor(
2524
@inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory,
@@ -30,35 +29,41 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
3029
@inject(IInterpreterService) protected readonly interpreterService: IInterpreterService,
3130
) {}
3231

33-
public async executeFile(file: Uri) {
32+
public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) {
3433
await this.setCwdForFileExecution(file);
3534
const { command, args } = await this.getExecuteFileArgs(file, [
3635
file.fsPath.fileToCommandArgumentForPythonExt(),
3736
]);
3837

39-
await this.getTerminalService(file).sendCommand(command, args);
38+
await this.getTerminalService(file, options).sendCommand(command, args);
4039
}
4140

4241
public async execute(code: string, resource?: Uri): Promise<void> {
4342
if (!code || code.trim().length === 0) {
4443
return;
4544
}
4645

47-
await this.initializeRepl();
46+
await this.initializeRepl(resource);
4847
await this.getTerminalService(resource).sendText(code);
4948
}
50-
public async initializeRepl(resource?: Uri) {
49+
public async initializeRepl(resource: Resource) {
50+
const terminalService = this.getTerminalService(resource);
5151
if (this.replActive && (await this.replActive)) {
52-
await this._terminalService.show();
52+
await terminalService.show();
5353
return;
5454
}
5555
this.replActive = new Promise<boolean>(async (resolve) => {
5656
const replCommandArgs = await this.getExecutableInfo(resource);
57-
await this.getTerminalService(resource).sendCommand(replCommandArgs.command, replCommandArgs.args);
57+
terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args);
5858

5959
// Give python repl time to start before we start sending text.
6060
setTimeout(() => resolve(true), 1000);
6161
});
62+
this.disposables.push(
63+
terminalService.onDidCloseTerminal(() => {
64+
this.replActive = undefined;
65+
}),
66+
);
6267

6368
await this.replActive;
6469
}
@@ -76,19 +81,12 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
7681
public async getExecuteFileArgs(resource?: Uri, executeArgs: string[] = []): Promise<PythonExecInfo> {
7782
return this.getExecutableInfo(resource, executeArgs);
7883
}
79-
private getTerminalService(resource?: Uri): ITerminalService {
80-
if (!this._terminalService) {
81-
this._terminalService = this.terminalServiceFactory.getTerminalService({
82-
resource,
83-
title: this.terminalTitle,
84-
});
85-
this.disposables.push(
86-
this._terminalService.onDidCloseTerminal(() => {
87-
this.replActive = undefined;
88-
}),
89-
);
90-
}
91-
return this._terminalService;
84+
private getTerminalService(resource: Resource, options?: { newTerminalPerFile: boolean }): ITerminalService {
85+
return this.terminalServiceFactory.getTerminalService({
86+
resource,
87+
title: this.terminalTitle,
88+
newTerminalPerFile: options?.newTerminalPerFile,
89+
});
9290
}
9391
private async setCwdForFileExecution(file: Uri) {
9492
const pythonSettings = this.configurationService.getSettings(file);

src/client/terminals/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const ICodeExecutionService = Symbol('ICodeExecutionService');
88

99
export interface ICodeExecutionService {
1010
execute(code: string, resource?: Uri): Promise<void>;
11-
executeFile(file: Uri): Promise<void>;
11+
executeFile(file: Uri, options?: { newTerminalPerFile: boolean }): Promise<void>;
1212
initializeRepl(resource?: Uri): Promise<void>;
1313
}
1414

src/test/common/terminals/factory.unit.test.ts

+46-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ suite('Terminal Service Factory', () => {
105105
expect(notSameAsThirdInstance).to.not.equal(true, 'Instances are the same');
106106
});
107107

108-
test('Ensure same terminal is returned when using resources from the same workspace', () => {
108+
test('Ensure same terminal is returned when using different resources from the same workspace', () => {
109109
const file1A = Uri.file('1a');
110110
const file2A = Uri.file('2a');
111111
const fileB = Uri.file('b');
@@ -140,4 +140,49 @@ suite('Terminal Service Factory', () => {
140140
'Instances should be different for different workspaces',
141141
);
142142
});
143+
144+
test('When `newTerminalPerFile` is true, ensure different terminal is returned when using different resources from the same workspace', () => {
145+
const file1A = Uri.file('1a');
146+
const file2A = Uri.file('2a');
147+
const fileB = Uri.file('b');
148+
const workspaceUriA = Uri.file('A');
149+
const workspaceUriB = Uri.file('B');
150+
const workspaceFolderA = TypeMoq.Mock.ofType<WorkspaceFolder>();
151+
workspaceFolderA.setup((w) => w.uri).returns(() => workspaceUriA);
152+
const workspaceFolderB = TypeMoq.Mock.ofType<WorkspaceFolder>();
153+
workspaceFolderB.setup((w) => w.uri).returns(() => workspaceUriB);
154+
155+
workspaceService
156+
.setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(file1A)))
157+
.returns(() => workspaceFolderA.object);
158+
workspaceService
159+
.setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(file2A)))
160+
.returns(() => workspaceFolderA.object);
161+
workspaceService
162+
.setup((w) => w.getWorkspaceFolder(TypeMoq.It.isValue(fileB)))
163+
.returns(() => workspaceFolderB.object);
164+
165+
const terminalForFile1A = factory.getTerminalService({
166+
resource: file1A,
167+
newTerminalPerFile: true,
168+
}) as SynchronousTerminalService;
169+
const terminalForFile2A = factory.getTerminalService({
170+
resource: file2A,
171+
newTerminalPerFile: true,
172+
}) as SynchronousTerminalService;
173+
const terminalForFileB = factory.getTerminalService({
174+
resource: fileB,
175+
newTerminalPerFile: true,
176+
}) as SynchronousTerminalService;
177+
178+
const terminalsAreSameForWorkspaceA = terminalForFile1A.terminalService === terminalForFile2A.terminalService;
179+
expect(terminalsAreSameForWorkspaceA).to.equal(false, 'Instances are the same for Workspace A');
180+
181+
const terminalsForWorkspaceABAreDifferent =
182+
terminalForFile1A.terminalService === terminalForFileB.terminalService;
183+
expect(terminalsForWorkspaceABAreDifferent).to.equal(
184+
false,
185+
'Instances should be different for different workspaces',
186+
);
187+
});
143188
});

0 commit comments

Comments
 (0)