From b61e2f196051808241689e402b38c396f61babfe Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 15:55:15 -0700 Subject: [PATCH 1/3] better executeCommand to prevent forever waiting --- src/client/common/terminal/service.ts | 49 +++++++++++++++++++++------ src/client/common/terminal/types.ts | 2 +- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 19cdf0aea0a1..6836bfab3360 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -99,19 +99,46 @@ export class TerminalService implements ITerminalService, Disposable { await promise; } + /** + * Cases we need to handle: + * + * Shell integration in zsh works, but not python + * + * 1. TODO: executeCommand never actually runs the command because the `python` command never finishes + * 2. onDidEndTerminalShellExecution never fires because because the `python` command never finishes + */ + 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 }); + // TODO: Test myself for windows + const execution = terminal.shellIntegration.executeCommand(commandLine); // create return object, on that object it has a promise to executed command + + // Before + // 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); + // } + // }); + + return { + execution, + exitCode: new Promise((resolve) => { + const listener = this.terminalManager.onDidEndTerminalShellExecution((e) => { + if (e.execution === execution) { + this.executeCommandListeners.delete(listener); + resolve(e.exitCode); + } + }); + if (listener) { + this.executeCommandListeners.add(listener); } - }); - if (listener) { - this.executeCommandListeners.add(listener); - } - }); + }), + }; } else { terminal.sendText(commandLine); } diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index aa8ff73cc205..46f4e1181f8c 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -60,7 +60,7 @@ export interface ITerminalService extends IDisposable { export interface ITerminalExecutedCommand { execution: TerminalShellExecution; - exitCode: number | undefined; + exitCode: Promise; } export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory'); From d1e6cee4bf4bb3418826343155439058a7fc73bd Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 17 Sep 2024 22:00:49 -0700 Subject: [PATCH 2/3] remove unused comments --- src/client/common/terminal/service.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 6836bfab3360..324e15088f2b 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -105,25 +105,11 @@ export class TerminalService implements ITerminalService, Disposable { * Shell integration in zsh works, but not python * * 1. TODO: executeCommand never actually runs the command because the `python` command never finishes - * 2. onDidEndTerminalShellExecution never fires because because the `python` command never finishes + * 2. onDidEndTerminalShellExecution never fires because because the `python` command never finishes --> complete */ if (terminal.shellIntegration) { - // TODO: Test myself for windows - const execution = terminal.shellIntegration.executeCommand(commandLine); // create return object, on that object it has a promise to executed command - - // Before - // 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); - // } - // }); + const execution = terminal.shellIntegration.executeCommand(commandLine); return { execution, From f66350a12cfafac4f8ab728b6001a3683e663f6d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 19 Sep 2024 00:29:02 -0700 Subject: [PATCH 3/3] comments --- src/client/common/terminal/service.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 324e15088f2b..18af2710b981 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -105,12 +105,14 @@ export class TerminalService implements ITerminalService, Disposable { * Shell integration in zsh works, but not python * * 1. TODO: executeCommand never actually runs the command because the `python` command never finishes - * 2. onDidEndTerminalShellExecution never fires because because the `python` command never finishes --> complete + * 2. onDidEndTerminalShellExecution never fires because because the `python` command never finishes --> line of defense via only promising on exitCode. */ if (terminal.shellIntegration) { const execution = terminal.shellIntegration.executeCommand(commandLine); + // To cover case #1, could we take any hint from execution (TerminalShellExecution) to know if executeCommand actually ran/sent anything? + return { execution, exitCode: new Promise((resolve) => {