Prevent first Python command being lost (microsoft#22902)

Fixes: microsoft#22673
Fixes: microsoft#22545
Fixes: microsoft#22691 

Making best effort to address issue where very first command sent to
REPL via Terminal gets ignored, or gets pasted both in Terminal and in
REPL.

With the fix, we observe whether Python REPL is launched in Terminal via
VS Code's `onDidWriteTerminalData` and send the command, or wait three
seconds as a fallback mechanism.

These two combined together will significantly reduce or resolve
all-together the chance of very first command being swollen up or gets
pasted twice in Terminal and REPL previously where it did not have
context of whether Python REPL instance have started inside the Terminal
or not.

Author: anthonykim1

src/client/terminals/codeExecution/djangoShellCodeExecution.ts

@@ -6,7 +6,12 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../common/application/types';
 import '../../common/extensions';
 import { IFileSystem, IPlatformService } from '../../common/platform/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
@@ -28,6 +33,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
         @inject(IFileSystem) fileSystem: IFileSystem,
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
         @inject(IInterpreterService) interpreterService: IInterpreterService,
+        @inject(IApplicationShell) applicationShell: IApplicationShell,
     ) {
         super(
             terminalServiceFactory,
@@ -37,6 +43,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
             platformService,
             interpreterService,
             commandManager,
+            applicationShell,
         );
         this.terminalTitle = 'Django Shell';
         disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));

src/client/terminals/codeExecution/repl.ts

@@ -5,7 +5,7 @@
 
 import { inject, injectable } from 'inversify';
 import { Disposable } from 'vscode';
-import { ICommandManager, IWorkspaceService } from '../../common/application/types';
+import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
@@ -22,6 +22,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
         @inject(IPlatformService) platformService: IPlatformService,
         @inject(IInterpreterService) interpreterService: IInterpreterService,
         @inject(ICommandManager) commandManager: ICommandManager,
+        @inject(IApplicationShell) applicationShell: IApplicationShell,
     ) {
         super(
             terminalServiceFactory,
@@ -31,6 +32,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
             platformService,
             interpreterService,
             commandManager,
+            applicationShell,
         );
         this.terminalTitle = 'REPL';
     }

src/client/terminals/codeExecution/terminalCodeExecution.ts

@@ -6,11 +6,11 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
-import { ICommandManager, IWorkspaceService } from '../../common/application/types';
+import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
-import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
+import { IConfigurationService, IDisposable, IDisposableRegistry, Resource } from '../../common/types';
 import { Diagnostics, Repl } from '../../common/utils/localize';
 import { showWarningMessage } from '../../common/vscodeApis/windowApis';
 import { IInterpreterService } from '../../interpreter/contracts';
@@ -30,6 +30,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         @inject(IPlatformService) protected readonly platformService: IPlatformService,
         @inject(IInterpreterService) protected readonly interpreterService: IInterpreterService,
         @inject(ICommandManager) protected readonly commandManager: ICommandManager,
+        @inject(IApplicationShell) protected readonly applicationShell: IApplicationShell,
     ) {}
 
     public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) {
@@ -65,10 +66,34 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         }
         this.replActive = new Promise<boolean>(async (resolve) => {
             const replCommandArgs = await this.getExecutableInfo(resource);
+            let listener: IDisposable;
+            Promise.race([
+                new Promise<boolean>((resolve) => setTimeout(() => resolve(true), 3000)),
+                new Promise<boolean>((resolve) => {
+                    let count = 0;
+                    const terminalDataTimeout = setTimeout(() => {
+                        resolve(true); // Fall back for test case scenarios.
+                    }, 3000);
+                    // Watch TerminalData to see if REPL launched.
+                    listener = this.applicationShell.onDidWriteTerminalData((e) => {
+                        for (let i = 0; i < e.data.length; i++) {
+                            if (e.data[i] === '>') {
+                                count++;
+                                if (count === 3) {
+                                    clearTimeout(terminalDataTimeout);
+                                    resolve(true);
+                                }
+                            }
+                        }
+                    });
+                }),
+            ]).then(() => {
+                if (listener) {
+                    listener.dispose();
+                }
+                resolve(true);
+            });
             terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args);
-
-            // Give python repl time to start before we start sending text.
-            setTimeout(() => resolve(true), 1000);
         });
         this.disposables.push(
             terminalService.onDidCloseTerminal(() => {

src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts

@@ -6,7 +6,12 @@ import * as path from 'path';
 import * as TypeMoq from 'typemoq';
 import * as sinon from 'sinon';
 import { Disposable, Uri, WorkspaceFolder } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../../client/common/application/types';
 import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
 import { createCondaEnv } from '../../../client/common/process/pythonEnvironment';
 import { createPythonProcessService } from '../../../client/common/process/pythonProcess';
@@ -32,12 +37,14 @@ suite('Terminal - Django Shell Code Execution', () => {
     let settings: TypeMoq.IMock<IPythonSettings>;
     let interpreterService: TypeMoq.IMock<IInterpreterService>;
     let pythonExecutionFactory: TypeMoq.IMock<IPythonExecutionFactory>;
+    let applicationShell: TypeMoq.IMock<IApplicationShell>;
     let disposables: Disposable[] = [];
     setup(() => {
         const terminalFactory = TypeMoq.Mock.ofType<ITerminalServiceFactory>();
         terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
         terminalService = TypeMoq.Mock.ofType<ITerminalService>();
         const configService = TypeMoq.Mock.ofType<IConfigurationService>();
+        applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
         workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
         workspace
             .setup((c) => c.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
@@ -62,6 +69,7 @@ suite('Terminal - Django Shell Code Execution', () => {
             fileSystem.object,
             disposables,
             interpreterService.object,
+            applicationShell.object,
         );
 
         terminalFactory.setup((f) => f.getTerminalService(TypeMoq.It.isAny())).returns(() => terminalService.object);

src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts

@@ -6,7 +6,12 @@ import * as path from 'path';
 import { SemVer } from 'semver';
 import * as TypeMoq from 'typemoq';
 import { Disposable, Uri, WorkspaceFolder } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../../client/common/application/types';
 import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
 import { createCondaEnv } from '../../../client/common/process/pythonEnvironment';
 import { createPythonProcessService } from '../../../client/common/process/pythonProcess';
@@ -47,6 +52,7 @@ suite('Terminal - Code Execution', () => {
         let pythonExecutionFactory: TypeMoq.IMock<IPythonExecutionFactory>;
         let interpreterService: TypeMoq.IMock<IInterpreterService>;
         let isDjangoRepl: boolean;
+        let applicationShell: TypeMoq.IMock<IApplicationShell>;
 
         teardown(() => {
             disposables.forEach((disposable) => {
@@ -71,6 +77,7 @@ suite('Terminal - Code Execution', () => {
             fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
             pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
             interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
+            applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
             settings = TypeMoq.Mock.ofType<IPythonSettings>();
             settings.setup((s) => s.terminal).returns(() => terminalSettings.object);
             configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
@@ -85,6 +92,7 @@ suite('Terminal - Code Execution', () => {
                         platform.object,
                         interpreterService.object,
                         commandManager.object,
+                        applicationShell.object,
                     );
                     break;
                 }
@@ -97,6 +105,7 @@ suite('Terminal - Code Execution', () => {
                         platform.object,
                         interpreterService.object,
                         commandManager.object,
+                        applicationShell.object,
                     );
                     expectedTerminalTitle = 'REPL';
                     break;
@@ -120,6 +129,7 @@ suite('Terminal - Code Execution', () => {
                         fileSystem.object,
                         disposables,
                         interpreterService.object,
+                        applicationShell.object,
                     );
                     expectedTerminalTitle = 'Django Shell';
                     break;
@@ -590,7 +600,7 @@ suite('Terminal - Code Execution', () => {
                 );
             });
 
-            test('Ensure repl is re-initialized when terminal is closed', async () => {
+            test('Ensure REPL launches after reducing risk of command being ignored or duplicated', async () => {
                 const pythonPath = 'usr/bin/python1234';
                 const terminalArgs = ['-a', 'b', 'c'];
                 platform.setup((p) => p.isWindows).returns(() => false);
@@ -599,43 +609,27 @@ suite('Terminal - Code Execution', () => {
                     .returns(() => Promise.resolve(({ path: pythonPath } as unknown) as PythonEnvironment));
                 terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs);
 
-                let closeTerminalCallback: undefined | (() => void);
-                terminalService
-                    .setup((t) => t.onDidCloseTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
-                    .returns((callback) => {
-                        closeTerminalCallback = callback;
-                        return {
-                            dispose: noop,
-                        };
-                    });
-
                 await executor.execute('cmd1');
                 await executor.execute('cmd2');
                 await executor.execute('cmd3');
 
-                const expectedTerminalArgs = isDjangoRepl ? terminalArgs.concat(['manage.py', 'shell']) : terminalArgs;
-
-                expect(closeTerminalCallback).not.to.be.an('undefined', 'Callback not initialized');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.once(),
+                // Now check if sendCommand from the initializeRepl is called atLeastOnce.
+                // This is due to newly added Promise race and fallback to lower risk of swollen first command.
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
 
-                closeTerminalCallback!.call(terminalService.object);
                 await executor.execute('cmd4');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.exactly(2),
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
 
-                closeTerminalCallback!.call(terminalService.object);
                 await executor.execute('cmd5');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.exactly(3),
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
             });