Skip to content

Commit 14bb14b

Browse files
committed
Switch over to executeCommand from sendText (microsoft#24078)
Resolves: microsoft#23929 TODO: (debt --> in separate PR) Have ensureTerminal return Promise<Terminal> instead of Promise<void> and saving this in the TerminalService class. Would avoid many uses of the !, and maybe even get to throw away the TerminalService class itself.
1 parent 127da14 commit 14bb14b

17 files changed

+233
-429
lines changed

.github/actions/smoke-tests/action.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ runs:
4343

4444
# Bits from the VSIX are reused by smokeTest.ts to speed things up.
4545
- name: Download VSIX
46-
uses: actions/download-artifact@v2
46+
uses: actions/download-artifact@v3
4747
with:
4848
name: ${{ inputs.artifact_name }}
4949

package-lock.json

+8-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
"theme": "dark"
4646
},
4747
"engines": {
48-
"vscode": "^1.91.0"
48+
"vscode": "^1.93.0"
4949
},
5050
"enableTelemetry": false,
5151
"keywords": [
@@ -1612,7 +1612,7 @@
16121612
"@types/sinon": "^17.0.3",
16131613
"@types/stack-trace": "0.0.29",
16141614
"@types/tmp": "^0.0.33",
1615-
"@types/vscode": "^1.81.0",
1615+
"@types/vscode": "^1.93.0",
16161616
"@types/which": "^2.0.1",
16171617
"@types/winreg": "^1.2.30",
16181618
"@types/xml2js": "^0.4.2",

src/client/common/application/terminalManager.ts

+16-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
// Licensed under the MIT License.
33

44
import { injectable } from 'inversify';
5-
import { Event, EventEmitter, Terminal, TerminalOptions, window } from 'vscode';
5+
import {
6+
Disposable,
7+
Event,
8+
EventEmitter,
9+
Terminal,
10+
TerminalOptions,
11+
TerminalShellExecutionEndEvent,
12+
TerminalShellIntegrationChangeEvent,
13+
window,
14+
} from 'vscode';
615
import { traceLog } from '../../logging';
716
import { ITerminalManager } from './types';
817

@@ -23,6 +32,12 @@ export class TerminalManager implements ITerminalManager {
2332
public createTerminal(options: TerminalOptions): Terminal {
2433
return monkeyPatchTerminal(window.createTerminal(options));
2534
}
35+
public onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable {
36+
return window.onDidChangeTerminalShellIntegration(handler);
37+
}
38+
public onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable {
39+
return window.onDidEndTerminalShellExecution(handler);
40+
}
2641
}
2742

2843
/**

src/client/common/application/types.ts

+6
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import {
4040
StatusBarItem,
4141
Terminal,
4242
TerminalOptions,
43+
TerminalShellExecutionEndEvent,
44+
TerminalShellIntegrationChangeEvent,
4345
TextDocument,
4446
TextDocumentChangeEvent,
4547
TextDocumentShowOptions,
@@ -936,6 +938,10 @@ export interface ITerminalManager {
936938
* @return A new Terminal.
937939
*/
938940
createTerminal(options: TerminalOptions): Terminal;
941+
942+
onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable;
943+
944+
onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable;
939945
}
940946

941947
export const IDebugService = Symbol('IDebugManager');

src/client/common/terminal/service.ts

+58-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ITerminalService,
2121
TerminalCreationOptions,
2222
TerminalShellType,
23+
ITerminalExecutedCommand,
2324
} from './types';
2425

2526
@injectable()
@@ -32,6 +33,7 @@ export class TerminalService implements ITerminalService, Disposable {
3233
private terminalActivator: ITerminalActivator;
3334
private terminalAutoActivator: ITerminalAutoActivation;
3435
private readonly envVarScript = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py');
36+
private readonly executeCommandListeners: Set<Disposable> = new Set();
3537
public get onDidCloseTerminal(): Event<void> {
3638
return this.terminalClosed.event.bind(this.terminalClosed);
3739
}
@@ -48,8 +50,12 @@ export class TerminalService implements ITerminalService, Disposable {
4850
this.terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
4951
}
5052
public dispose() {
51-
if (this.terminal) {
52-
this.terminal.dispose();
53+
this.terminal?.dispose();
54+
55+
if (this.executeCommandListeners && this.executeCommandListeners.size > 0) {
56+
this.executeCommandListeners.forEach((d) => {
57+
d?.dispose();
58+
});
5359
}
5460
}
5561
public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise<void> {
@@ -59,21 +65,67 @@ export class TerminalService implements ITerminalService, Disposable {
5965
this.terminal!.show(true);
6066
}
6167

62-
this.terminal!.sendText(text, true);
68+
await this.executeCommand(text);
6369
}
70+
/** @deprecated */
6471
public async sendText(text: string): Promise<void> {
6572
await this.ensureTerminal();
6673
if (!this.options?.hideFromUser) {
6774
this.terminal!.show(true);
6875
}
6976
this.terminal!.sendText(text);
7077
}
78+
public async executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
79+
const terminal = this.terminal!;
80+
if (!this.options?.hideFromUser) {
81+
terminal.show(true);
82+
}
83+
84+
// If terminal was just launched, wait some time for shell integration to onDidChangeShellIntegration.
85+
if (!terminal.shellIntegration) {
86+
const promise = new Promise<boolean>((resolve) => {
87+
const shellIntegrationChangeEventListener = this.terminalManager.onDidChangeTerminalShellIntegration(
88+
() => {
89+
this.executeCommandListeners.delete(shellIntegrationChangeEventListener);
90+
resolve(true);
91+
},
92+
);
93+
const TIMEOUT_DURATION = 3000;
94+
setTimeout(() => {
95+
this.executeCommandListeners.add(shellIntegrationChangeEventListener);
96+
resolve(true);
97+
}, TIMEOUT_DURATION);
98+
});
99+
await promise;
100+
}
101+
102+
if (terminal.shellIntegration) {
103+
const execution = terminal.shellIntegration.executeCommand(commandLine);
104+
return await new Promise((resolve) => {
105+
const listener = this.terminalManager.onDidEndTerminalShellExecution((e) => {
106+
if (e.execution === execution) {
107+
this.executeCommandListeners.delete(listener);
108+
resolve({ execution, exitCode: e.exitCode });
109+
}
110+
});
111+
if (listener) {
112+
this.executeCommandListeners.add(listener);
113+
}
114+
});
115+
} else {
116+
terminal.sendText(commandLine);
117+
}
118+
119+
return undefined;
120+
}
121+
71122
public async show(preserveFocus: boolean = true): Promise<void> {
72123
await this.ensureTerminal(preserveFocus);
73124
if (!this.options?.hideFromUser) {
74125
this.terminal!.show(preserveFocus);
75126
}
76127
}
128+
// TODO: Debt switch to Promise<Terminal> ---> breaks 20 tests
77129
public async ensureTerminal(preserveFocus: boolean = true): Promise<void> {
78130
if (this.terminal) {
79131
return;
@@ -89,18 +141,19 @@ export class TerminalService implements ITerminalService, Disposable {
89141
// Sometimes the terminal takes some time to start up before it can start accepting input.
90142
await new Promise((resolve) => setTimeout(resolve, 100));
91143

92-
await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, {
144+
await this.terminalActivator.activateEnvironmentInTerminal(this.terminal, {
93145
resource: this.options?.resource,
94146
preserveFocus,
95147
interpreter: this.options?.interpreter,
96148
hideFromUser: this.options?.hideFromUser,
97149
});
98150

99151
if (!this.options?.hideFromUser) {
100-
this.terminal!.show(preserveFocus);
152+
this.terminal.show(preserveFocus);
101153
}
102154

103155
this.sendTelemetry().ignoreErrors();
156+
return;
104157
}
105158
private terminalCloseHandler(terminal: Terminal) {
106159
if (terminal === this.terminal) {

src/client/common/terminal/syncTerminalService.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts';
1414
import { createDeferred, Deferred } from '../utils/async';
1515
import { noop } from '../utils/misc';
1616
import { TerminalService } from './service';
17-
import { ITerminalService } from './types';
17+
import { ITerminalService, ITerminalExecutedCommand } from './types';
1818

1919
enum State {
2020
notStarted = 0,
@@ -146,9 +146,13 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
146146
lockFile.dispose();
147147
}
148148
}
149+
/** @deprecated */
149150
public sendText(text: string): Promise<void> {
150151
return this.terminalService.sendText(text);
151152
}
153+
public executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
154+
return this.terminalService.executeCommand(commandLine);
155+
}
152156
public show(preserveFocus?: boolean | undefined): Promise<void> {
153157
return this.terminalService.show(preserveFocus);
154158
}

src/client/common/terminal/types.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
'use strict';
55

6-
import { CancellationToken, Event, Terminal, Uri } from 'vscode';
6+
import { CancellationToken, Event, Terminal, Uri, TerminalShellExecution } from 'vscode';
77
import { PythonEnvironment } from '../../pythonEnvironments/info';
88
import { IEventNamePropertyMapping } from '../../telemetry/index';
99
import { IDisposable, Resource } from '../types';
@@ -52,10 +52,17 @@ export interface ITerminalService extends IDisposable {
5252
cancel?: CancellationToken,
5353
swallowExceptions?: boolean,
5454
): Promise<void>;
55+
/** @deprecated */
5556
sendText(text: string): Promise<void>;
57+
executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined>;
5658
show(preserveFocus?: boolean): Promise<void>;
5759
}
5860

61+
export interface ITerminalExecutedCommand {
62+
execution: TerminalShellExecution;
63+
exitCode: number | undefined;
64+
}
65+
5966
export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory');
6067

6168
export type TerminalCreationOptions = {

src/client/terminals/codeExecution/terminalCodeExecution.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
6060
this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource);
6161
}
6262
} else {
63-
await this.getTerminalService(resource).sendText(code);
63+
await this.getTerminalService(resource).executeCommand(code);
6464
}
6565
}
6666

0 commit comments

Comments
 (0)