repl: fix multiline history editing string order

Author: puskin

lib/internal/readline/interface.js

@@ -467,9 +467,15 @@ class Interface extends InterfaceConstructor {
   }
 
   // Convert newlines to a consistent format for history storage
-  [kNormalizeHistoryLineEndings](line, from, to) {
+  [kNormalizeHistoryLineEndings](line, from, to, reverse = true) {
     // Multiline history entries are saved reversed
-    if (StringPrototypeIncludes(line, '\r')) {
+    // History is structured with the newest entries at the top
+    // and the oldest at the bottom. Multiline histories, however, only occupy
+    // one line in the history file. When loading multiline history with
+    // an old node binary, the history will be saved in the old format.
+    // This is why we need to reverse the multilines.
+    // Reversing the multilines is necessary when adding / editing and displaying them
+    if (reverse) {
       // First reverse the lines for proper order, then convert separators
       return ArrayPrototypeJoin(
         ArrayPrototypeReverse(StringPrototypeSplit(line, from)),
@@ -488,7 +494,7 @@ class Interface extends InterfaceConstructor {
 
     // If the trimmed line is empty then return the line
     if (StringPrototypeTrim(this.line).length === 0) return this.line;
-    const normalizedLine = this[kNormalizeHistoryLineEndings](this.line, '\n', '\r');
+    const normalizedLine = this[kNormalizeHistoryLineEndings](this.line, '\n', '\r', false);
 
     if (this.history.length === 0 || this.history[0] !== normalizedLine) {
       if (this[kLastCommandErrored] && this.historyIndex === 0) {

test/parallel/test-repl-multiline-navigation.js

@@ -8,6 +8,7 @@ const assert = require('assert');
 const repl = require('internal/repl');
 const stream = require('stream');
 const fs = require('fs');
+const { setTimeout } = require('timers/promises');
 
 class ActionStream extends stream.Stream {
   run(data) {
@@ -53,6 +54,23 @@ function cleanupTmpFile() {
   return true;
 }
 
+// Helper function to wait for a condition with a retry mechanism
+// This should fix flakiness in the REPL tests, due to its nature
+const kMaxRetries = 3;
+async function pollForCondition(assertionFn) {
+  for (let retries = 0; retries <= kMaxRetries; retries++) {
+    try {
+      assertionFn();
+      return;
+    } catch (err) {
+      if (retries === kMaxRetries) {
+        throw err;
+      }
+      await setTimeout(50);
+    }
+  }
+}
+
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
 const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
@@ -62,7 +80,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
   // Make sure the cursor is at the right places.
   // If the cursor is at the end of a long line and the down key is pressed,
   // Move the cursor to the end of the next line, if shorter.
-  const checkResults = common.mustSucceed((r) => {
+  const checkResults = common.mustSucceed(async (r) => {
     r.write('let str = `');
     r.input.run([{ name: 'enter' }]);
     r.write('111');
@@ -72,28 +90,28 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
     r.write('3`');
     r.input.run([{ name: 'enter' }]);
     r.input.run([{ name: 'up' }]);
-    assert.strictEqual(r.cursor, 33);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 33));
 
     r.input.run([{ name: 'up' }]);
-    assert.strictEqual(r.cursor, 18);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 18));
 
     for (let i = 0; i < 5; i++) {
       r.input.run([{ name: 'right' }]);
     }
     r.input.run([{ name: 'up' }]);
-    assert.strictEqual(r.cursor, 15);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 15));
     r.input.run([{ name: 'up' }]);
 
     for (let i = 0; i < 5; i++) {
       r.input.run([{ name: 'right' }]);
     }
-    assert.strictEqual(r.cursor, 8);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 8));
 
     r.input.run([{ name: 'down' }]);
-    assert.strictEqual(r.cursor, 15);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 15));
 
     r.input.run([{ name: 'down' }]);
-    assert.strictEqual(r.cursor, 19);
+    await pollForCondition(() => assert.strictEqual(r.cursor, 19));
   });
 
   repl.createInternalRepl(
@@ -115,7 +133,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
   cleanupTmpFile();
   // If the last command errored and the user is trying to edit it,
   // The errored line should be removed from history
-  const checkResults = common.mustSucceed((r) => {
+  const checkResults = common.mustSucceed(async (r) => {
     r.write('let lineWithMistake = `I have some');
     r.input.run([{ name: 'enter' }]);
     r.write('problem with` my syntax\'');
@@ -129,9 +147,18 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
     r.input.run([{ name: 'backspace' }]);
     r.input.run([{ name: 'enter' }]);
 
-    assert.strictEqual(r.history.length, 1);
-    assert.strictEqual(r.history[0], 'let lineWithMistake = `I have some\rproblem with my syntax`');
-    assert.strictEqual(r.line, '');
+    await pollForCondition(() => assert.strictEqual(r.history.length, 1));
+    // Check that the line is properly set in the history structure
+    await pollForCondition(() => assert.strictEqual(r.history[0],
+                                                    'problem with my syntax`\rlet lineWithMistake = `I have some')
+    );
+    await pollForCondition(() => assert.strictEqual(r.line, ''));
+
+    r.input.run([{ name: 'up' }]);
+    // Check that the line is properly displayed
+    await pollForCondition(() => assert.strictEqual(r.line,
+                                                    'let lineWithMistake = `I have some\nproblem with my syntax`')
+    );
   });
 
   repl.createInternalRepl(
@@ -155,7 +182,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
 
   // Test that the REPL preview is properly shown on multiline commands
   // And deleted when enter is pressed
-  const checkResults = common.mustSucceed((r) => {
+  const checkResults = common.mustSucceed(async (r) => {
     r.write('Array(100).fill(');
     r.input.run([{ name: 'enter' }]);
     r.write('123');
@@ -165,9 +192,9 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
     r.input.run([{ name: 'up' }]);
     r.input.run([{ name: 'up' }]);
 
-    assert.deepStrictEqual(r.last, new Array(100).fill(123));
+    await pollForCondition(() => assert.deepStrictEqual(r.last, new Array(100).fill(123)));
     r.input.run([{ name: 'enter' }]);
-    assert.strictEqual(outputBuffer.includes('[\n' +
+    await pollForCondition(() => assert.strictEqual(outputBuffer.includes('[\n' +
     '  123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' +
     '  123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' +
     '  123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' +
@@ -178,7 +205,7 @@ const defaultHistoryPath = tmpdir.resolve('.node_repl_history');
     '  123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' +
     '  123, 123, 123, 123, 123, 123, 123, 123, 123, 123, 123,\n' +
     '  123\n' +
-    ']\n'), true);
+    ']\n'), true));
   });
 
   repl.createInternalRepl(