fix(cli-repl): aggregate regular characters in LineByLineInput MONGOS…

packages/cli-repl/src/async-repl.ts

@@ -1,6 +1,6 @@
 /* eslint-disable chai-friendly/no-unused-expressions */
 import type { REPLServer, ReplOptions } from 'repl';
-import type { ReadLineOptions } from 'readline';
+import { Interface, ReadLineOptions } from 'readline';
 import type { EventEmitter } from 'events';
 import { Recoverable, start as originalStart } from 'repl';
 import isRecoverableError from 'is-recoverable-error';
@@ -71,7 +71,20 @@ export function start(opts: AsyncREPLOptions): REPLServer {
 
     // Use public getPrompt() API once available (Node.js 15+)
     const origPrompt = getPrompt(repl);
-    repl.setPrompt(''); // Disable printing prompts while we're evaluating code.
+    // Disable printing prompts while we're evaluating code. We're using the
+    // readline superclass method instead of the REPL one here, because the REPL
+    // one stores the prompt to later be reset in case of dropping into .editor
+    // mode. In particular, the following sequence of results is what we want
+    // to avoid:
+    // 1. .editor entered
+    // 2. Some code entered
+    // 3. Tab used for autocompletion, leading to this evaluation being called
+    //    while the REPL prompt is still turned off due to .editor
+    // 4. Evaluation ends, we use .setPrompt() to restore the prompt that has
+    //    temporarily been disable for .editor
+    // 5. The REPL thinks that the empty string is supposed to be the prompt
+    //    even after .editor is done.
+    Interface.prototype.setPrompt.call(repl, '');
 
     try {
       let exitEventPending = false;
@@ -126,7 +139,7 @@ export function start(opts: AsyncREPLOptions): REPLServer {
         (processSigint as any)?.restore?.();
 
         if (getPrompt(repl) === '') {
-          repl.setPrompt(origPrompt);
+          Interface.prototype.setPrompt.call(repl, origPrompt);
         }
 
         repl.removeListener('exit', exitListener);

packages/cli-repl/src/cli-repl.spec.ts

@@ -6,7 +6,7 @@ import path from 'path';
 import { Duplex, PassThrough } from 'stream';
 import { promisify } from 'util';
 import { MongodSetup, startTestServer } from '../../../testing/integration-testing-hooks';
-import { expect, fakeTTYProps, readReplLogfile, useTmpdir, waitBus, waitCompletion, waitEval } from '../test/repl-helpers';
+import { expect, fakeTTYProps, readReplLogfile, useTmpdir, waitBus, waitCompletion, waitEval, tick } from '../test/repl-helpers';
 import CliRepl, { CliReplOptions } from './cli-repl';
 import { CliReplErrors } from './error-codes';
 
@@ -818,7 +818,14 @@ describe('CliRepl', () => {
   }): void {
     describe('autocompletion', () => {
       let cliRepl: CliRepl;
-      const tab = '\u0009';
+      const tab = async() => {
+        await tick();
+        input.write('\u0009');
+      };
+      const tabtab = async() => {
+        await tab();
+        await tab();
+      };
 
       beforeEach(async() => {
         if (testServer === null) {
@@ -837,7 +844,8 @@ describe('CliRepl', () => {
 
       it(`${wantWatch ? 'completes' : 'does not complete'} the watch method`, async() => {
         output = '';
-        input.write(`db.wat${tab}${tab}`);
+        input.write('db.wat');
+        await tabtab();
         await waitCompletion(cliRepl.bus);
         if (wantWatch) {
           expect(output).to.include('db.watch');
@@ -848,14 +856,16 @@ describe('CliRepl', () => {
 
       it('completes the version method', async() => {
         output = '';
-        input.write(`db.vers${tab}${tab}`);
+        input.write('db.vers');
+        await tabtab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('db.version');
       });
 
       it(`${wantShardDistribution ? 'completes' : 'does not complete'} the getShardDistribution method`, async() => {
         output = '';
-        input.write(`db.coll.getShardDis${tab}${tab}`);
+        input.write('db.coll.getShardDis');
+        await tabtab();
         await waitCompletion(cliRepl.bus);
         if (wantShardDistribution) {
           expect(output).to.include('db.coll.getShardDistribution');
@@ -871,7 +881,8 @@ describe('CliRepl', () => {
         await waitEval(cliRepl.bus);
 
         output = '';
-        input.write(`db.testcoll${tab}${tab}`);
+        input.write('db.testcoll');
+        await tabtab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include(collname);
 
@@ -880,15 +891,17 @@ describe('CliRepl', () => {
       });
 
       it('completes JS value properties properly (incomplete, double tab)', async() => {
-        input.write(`JSON.${tab}${tab}`);
+        input.write('JSON.');
+        await tabtab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('JSON.parse');
         expect(output).to.include('JSON.stringify');
         expect(output).not.to.include('rawValue');
       });
 
       it('completes JS value properties properly (complete, single tab)', async() => {
-        input.write(`JSON.pa${tab}`);
+        input.write('JSON.pa');
+        await tab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('JSON.parse');
         expect(output).not.to.include('JSON.stringify');
@@ -900,15 +913,17 @@ describe('CliRepl', () => {
         await waitEval(cliRepl.bus);
 
         output = '';
-        input.write(`show d${tab}`);
+        input.write('show d');
+        await tab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('show databases');
         expect(output).not.to.include('dSomeVariableStartingWithD');
       });
 
       it('completes use <db>', async() => {
         if (!hasDatabaseNames) return;
-        input.write(`use adm${tab}`);
+        input.write('use adm');
+        await tab();
         await waitCompletion(cliRepl.bus);
         expect(output).to.include('use admin');
       });

packages/cli-repl/src/line-by-line-input.spec.ts

@@ -27,7 +27,7 @@ describe('LineByLineInput', () => {
   context('when block on newline is enabled (default)', () => {
     it('does not forward characters after newline', () => {
       stdinMock.emit('data', Buffer.from('ab\nc'));
-      expect(forwardedChunks).to.deep.equal(['a', 'b', '\n']);
+      expect(forwardedChunks).to.deep.equal(['ab', '\n']);
     });
 
     it('forwards CTRL-C anyway and as soon as is received', () => {
@@ -43,7 +43,7 @@ describe('LineByLineInput', () => {
     it('unblocks on nextline', () => {
       stdinMock.emit('data', Buffer.from('ab\nc'));
       lineByLineInput.nextLine();
-      expect(forwardedChunks).to.deep.equal(['a', 'b', '\n', 'c']);
+      expect(forwardedChunks).to.deep.equal(['ab', '\n', 'c']);
     });
   });
 
@@ -60,7 +60,7 @@ describe('LineByLineInput', () => {
       lineByLineInput.disableBlockOnNewline();
       lineByLineInput.enableBlockOnNewLine();
       stdinMock.emit('data', Buffer.from('ab\nc'));
-      expect(forwardedChunks).to.deep.equal(['a', 'b', '\n']);
+      expect(forwardedChunks).to.deep.equal(['ab', '\n']);
     });
   });
 
@@ -77,8 +77,8 @@ describe('LineByLineInput', () => {
         insideDataCalls--;
       });
       stdinMock.emit('data', Buffer.from('foo\n\u0003'));
-      expect(dataCalls).to.equal(5);
-      expect(forwardedChunks).to.deep.equal(['\u0003', 'f', 'o', 'o', '\n']);
+      expect(dataCalls).to.equal(3);
+      expect(forwardedChunks).to.deep.equal(['\u0003', 'foo', '\n']);
     });
   });
 });

packages/cli-repl/src/line-by-line-input.ts

@@ -105,6 +105,11 @@ export class LineByLineInput extends Readable {
     for (const char of chars) {
       if (this._isCtrlC(char) || this._isCtrlD(char)) {
         this.push(char);
+      } else if (
+        this._isRegularCharacter(char) &&
+        this._charQueue.length > 0 &&
+        this._isRegularCharacter(this._charQueue[this._charQueue.length - 1])) {
+        (this._charQueue[this._charQueue.length - 1] as string) += char as string;
       } else {
         this._charQueue.push(char);
       }
@@ -162,6 +167,10 @@ export class LineByLineInput extends Readable {
     }
   }
 
+  private _isRegularCharacter(char: string | null): boolean {
+    return char !== null && !this._isLineEnding(char) && !this._isCtrlC(char) && !this._isCtrlD(char);
+  }
+
   private _isLineEnding(char: string | null): boolean {
     return char !== null && LINE_ENDING_RE.test(char);
   }

packages/cli-repl/src/mongosh-repl.spec.ts

@@ -223,7 +223,14 @@ describe('MongoshNodeRepl', () => {
   });
 
   context('with terminal: true', () => {
-    const tab = '\u0009';
+    const tab = async() => {
+      await tick();
+      input.write('\u0009');
+    };
+    const tabtab = async() => {
+      await tab();
+      await tab();
+    };
 
     beforeEach(async() => {
       // Node.js uses $TERM to determine what level of functionality to provide
@@ -254,7 +261,8 @@ describe('MongoshNodeRepl', () => {
       await tick();
       expect(output).to.include('Entering editor mode');
       output = '';
-      input.write(`db.${tab}${tab}`);
+      input.write('db.');
+      await tabtab();
       await tick();
       input.write('version()\n');
       input.write('\u0004'); // Ctrl+D
@@ -298,30 +306,30 @@ describe('MongoshNodeRepl', () => {
 
     context('autocompletion', () => {
       it('autocompletes collection methods', async() => {
-        // this triggers an eval
-        input.write(`db.coll.${tab}${tab}`);
-        // first tab
-        await waitEval(bus);
-        // second tab
+        input.write('db.coll.');
+        await tabtab();
         await tick();
         expect(output).to.include('db.coll.updateOne');
       });
       it('autocompletes shell-api methods (once)', async() => {
-        input.write(`vers${tab}${tab}`);
+        input.write('vers');
+        await tabtab();
         await tick();
         expect(output).to.include('version');
         expect(output).to.not.match(/version[ \t]+version/);
       });
       it('autocompletes async shell api methods', async() => {
-        input.write(`db.coll.find().${tab}${tab}`);
+        input.write('db.coll.find().');
+        await tabtab();
         await tick();
         expect(output).to.include('db.coll.find().close');
       });
       it('autocompletes local variables', async() => {
         input.write('let somelongvariable = 0\n');
         await waitEval(bus);
         output = '';
-        input.write(`somelong${tab}${tab}`);
+        input.write('somelong');
+        await tabtab();
         await tick();
         expect(output).to.include('somelongvariable');
       });
@@ -330,14 +338,21 @@ describe('MongoshNodeRepl', () => {
         await tick();
         output = '';
         expect((mongoshRepl.runtimeState().repl as any)._prompt).to.equal('');
-        input.write(`db.${tab}${tab}`);
+        input.write('db.');
+        await tabtab();
         await tick();
         input.write('foo\nbar\n');
         expect((mongoshRepl.runtimeState().repl as any)._prompt).to.equal('');
         input.write('\u0003'); // Ctrl+C for abort
         await tick();
         expect((mongoshRepl.runtimeState().repl as any)._prompt).to.equal('> ');
-        expect(stripAnsi(output)).to.equal('db.\tfoo\r\nbar\r\n\r\n> ');
+        expect(stripAnsi(output)).to.equal('db.foo\r\nbar\r\n\r\n> ');
+      });
+      it('does not autocomplete tab-indented code', async() => {
+        output = '';
+        input.write('\t\tfoo');
+        await tick();
+        expect(output).to.equal('\t\tfoo');
       });
     });
 

packages/cli-repl/test/e2e-direct.spec.ts

@@ -6,6 +6,13 @@ import { TestShell } from './test-shell';
 describe('e2e direct connection', () => {
   afterEach(TestShell.cleanup);
 
+  const tabtab = async(shell: TestShell) => {
+    await new Promise(resolve => setTimeout(resolve, 400));
+    shell.writeInput('\u0009');
+    await new Promise(resolve => setTimeout(resolve, 400));
+    shell.writeInput('\u0009');
+  };
+
   context('to a replica set', async() => {
     const replSetId = 'replset';
     const [rs0, rs1, rs2] = startTestCluster(
@@ -120,7 +127,8 @@ describe('e2e direct connection', () => {
           }
           const shell = TestShell.start({ args: [`${await rs1.connectionString()}/${dbname}`], forceTerminal: true });
           await shell.waitForPrompt();
-          shell.writeInput('db.testc\u0009\u0009');
+          shell.writeInput('db.testc');
+          await tabtab(shell);
           await eventually(() => {
             shell.assertContainsOutput('db.testcollection');
           });
@@ -195,7 +203,8 @@ describe('e2e direct connection', () => {
           }
           const shell = TestShell.start({ args: [`${await rs1.connectionString()}/${dbname}`], forceTerminal: true });
           await shell.waitForPrompt();
-          shell.writeInput('db.testc\u0009\u0009');
+          shell.writeInput('db.testc');
+          await tabtab(shell);
           await eventually(() => {
             shell.assertContainsOutput('db.testcollection');
           });