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.
anthonykim1 · Sep 12, 2024 · 216c7ed · 216c7ed
1 parent 6578d9d
commit 216c7ed
Show file tree

Hide file tree

Showing 17 changed files with 233 additions and 429 deletions.
diff --git a/.github/actions/smoke-tests/action.yml b/.github/actions/smoke-tests/action.yml
@@ -43,7 +43,7 @@ runs:
 
     # Bits from the VSIX are reused by smokeTest.ts to speed things up.
     - name: Download VSIX
-      uses: actions/download-artifact@v2
+      uses: actions/download-artifact@v3
       with:
         name: ${{ inputs.artifact_name }}
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -45,7 +45,7 @@
         "theme": "dark"
     },
     "engines": {
-        "vscode": "^1.91.0"
+        "vscode": "^1.93.0"
     },
     "enableTelemetry": false,
     "keywords": [
@@ -1570,7 +1570,7 @@
         "@types/sinon": "^17.0.3",
         "@types/stack-trace": "0.0.29",
         "@types/tmp": "^0.0.33",
-        "@types/vscode": "^1.81.0",
+        "@types/vscode": "^1.93.0",
         "@types/which": "^2.0.1",
         "@types/winreg": "^1.2.30",
         "@types/xml2js": "^0.4.2",

diff --git a/src/client/common/application/terminalManager.ts b/src/client/common/application/terminalManager.ts
@@ -2,7 +2,16 @@
 // Licensed under the MIT License.
 
 import { injectable } from 'inversify';
-import { Event, EventEmitter, Terminal, TerminalOptions, window } from 'vscode';
+import {
+    Disposable,
+    Event,
+    EventEmitter,
+    Terminal,
+    TerminalOptions,
+    TerminalShellExecutionEndEvent,
+    TerminalShellIntegrationChangeEvent,
+    window,
+} from 'vscode';
 import { traceLog } from '../../logging';
 import { ITerminalManager } from './types';
 
@@ -23,6 +32,12 @@ export class TerminalManager implements ITerminalManager {
     public createTerminal(options: TerminalOptions): Terminal {
         return monkeyPatchTerminal(window.createTerminal(options));
     }
+    public onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable {
+        return window.onDidChangeTerminalShellIntegration(handler);
+    }
+    public onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable {
+        return window.onDidEndTerminalShellExecution(handler);
+    }
 }
 
 /**

diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts
@@ -40,6 +40,8 @@ import {
     StatusBarItem,
     Terminal,
     TerminalOptions,
+    TerminalShellExecutionEndEvent,
+    TerminalShellIntegrationChangeEvent,
     TextDocument,
     TextDocumentChangeEvent,
     TextDocumentShowOptions,
@@ -936,6 +938,10 @@ export interface ITerminalManager {
      * @return A new Terminal.
      */
     createTerminal(options: TerminalOptions): Terminal;
+
+    onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable;
+
+    onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable;
 }
 
 export const IDebugService = Symbol('IDebugManager');

diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts
@@ -20,6 +20,7 @@ import {
     ITerminalService,
     TerminalCreationOptions,
     TerminalShellType,
+    ITerminalExecutedCommand,
 } from './types';
 
 @injectable()
@@ -32,6 +33,7 @@ export class TerminalService implements ITerminalService, Disposable {
     private terminalActivator: ITerminalActivator;
     private terminalAutoActivator: ITerminalAutoActivation;
     private readonly envVarScript = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py');
+    private readonly executeCommandListeners: Set<Disposable> = new Set();
     public get onDidCloseTerminal(): Event<void> {
         return this.terminalClosed.event.bind(this.terminalClosed);
     }
@@ -48,8 +50,12 @@ export class TerminalService implements ITerminalService, Disposable {
         this.terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
     }
     public dispose() {
-        if (this.terminal) {
-            this.terminal.dispose();
+        this.terminal?.dispose();
+
+        if (this.executeCommandListeners && this.executeCommandListeners.size > 0) {
+            this.executeCommandListeners.forEach((d) => {
+                d?.dispose();
+            });
         }
     }
     public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise<void> {
@@ -59,21 +65,67 @@ export class TerminalService implements ITerminalService, Disposable {
             this.terminal!.show(true);
         }
 
-        this.terminal!.sendText(text, true);
+        await this.executeCommand(text);
     }
+    /** @deprecated */
     public async sendText(text: string): Promise<void> {
         await this.ensureTerminal();
         if (!this.options?.hideFromUser) {
             this.terminal!.show(true);
         }
         this.terminal!.sendText(text);
     }
+    public async executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
+        const terminal = this.terminal!;
+        if (!this.options?.hideFromUser) {
+            terminal.show(true);
+        }
+
+        // If terminal was just launched, wait some time for shell integration to onDidChangeShellIntegration.
+        if (!terminal.shellIntegration) {
+            const promise = new Promise<boolean>((resolve) => {
+                const shellIntegrationChangeEventListener = this.terminalManager.onDidChangeTerminalShellIntegration(
+                    () => {
+                        this.executeCommandListeners.delete(shellIntegrationChangeEventListener);
+                        resolve(true);
+                    },
+                );
+                const TIMEOUT_DURATION = 3000;
+                setTimeout(() => {
+                    this.executeCommandListeners.add(shellIntegrationChangeEventListener);
+                    resolve(true);
+                }, TIMEOUT_DURATION);
+            });
+            await promise;
+        }
+
+        if (terminal.shellIntegration) {
+            const execution = terminal.shellIntegration.executeCommand(commandLine);
+            return await new Promise((resolve) => {
+                const listener = this.terminalManager.onDidEndTerminalShellExecution((e) => {
+                    if (e.execution === execution) {
+                        this.executeCommandListeners.delete(listener);
+                        resolve({ execution, exitCode: e.exitCode });
+                    }
+                });
+                if (listener) {
+                    this.executeCommandListeners.add(listener);
+                }
+            });
+        } else {
+            terminal.sendText(commandLine);
+        }
+
+        return undefined;
+    }
+
     public async show(preserveFocus: boolean = true): Promise<void> {
         await this.ensureTerminal(preserveFocus);
         if (!this.options?.hideFromUser) {
             this.terminal!.show(preserveFocus);
         }
     }
+    // TODO: Debt switch to Promise<Terminal> ---> breaks 20 tests
     public async ensureTerminal(preserveFocus: boolean = true): Promise<void> {
         if (this.terminal) {
             return;
@@ -89,18 +141,19 @@ export class TerminalService implements ITerminalService, Disposable {
         // Sometimes the terminal takes some time to start up before it can start accepting input.
         await new Promise((resolve) => setTimeout(resolve, 100));
 
-        await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, {
+        await this.terminalActivator.activateEnvironmentInTerminal(this.terminal, {
             resource: this.options?.resource,
             preserveFocus,
             interpreter: this.options?.interpreter,
             hideFromUser: this.options?.hideFromUser,
         });
 
         if (!this.options?.hideFromUser) {
-            this.terminal!.show(preserveFocus);
+            this.terminal.show(preserveFocus);
         }
 
         this.sendTelemetry().ignoreErrors();
+        return;
     }
     private terminalCloseHandler(terminal: Terminal) {
         if (terminal === this.terminal) {

diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts
@@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts';
 import { createDeferred, Deferred } from '../utils/async';
 import { noop } from '../utils/misc';
 import { TerminalService } from './service';
-import { ITerminalService } from './types';
+import { ITerminalService, ITerminalExecutedCommand } from './types';
 
 enum State {
     notStarted = 0,
@@ -146,9 +146,13 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
             lockFile.dispose();
         }
     }
+    /** @deprecated */
     public sendText(text: string): Promise<void> {
         return this.terminalService.sendText(text);
     }
+    public executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined> {
+        return this.terminalService.executeCommand(commandLine);
+    }
     public show(preserveFocus?: boolean | undefined): Promise<void> {
         return this.terminalService.show(preserveFocus);
     }

diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts
@@ -3,7 +3,7 @@
 
 'use strict';
 
-import { CancellationToken, Event, Terminal, Uri } from 'vscode';
+import { CancellationToken, Event, Terminal, Uri, TerminalShellExecution } from 'vscode';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
 import { IEventNamePropertyMapping } from '../../telemetry/index';
 import { IDisposable, Resource } from '../types';
@@ -52,10 +52,17 @@ export interface ITerminalService extends IDisposable {
         cancel?: CancellationToken,
         swallowExceptions?: boolean,
     ): Promise<void>;
+    /** @deprecated */
     sendText(text: string): Promise<void>;
+    executeCommand(commandLine: string): Promise<ITerminalExecutedCommand | undefined>;
     show(preserveFocus?: boolean): Promise<void>;
 }
 
+export interface ITerminalExecutedCommand {
+    execution: TerminalShellExecution;
+    exitCode: number | undefined;
+}
+
 export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory');
 
 export type TerminalCreationOptions = {

diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts
@@ -59,7 +59,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
                 this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource);
             }
         } else {
-            await this.getTerminalService(resource).sendText(code);
+            await this.getTerminalService(resource).executeCommand(code);
         }
     }