repl: add proper vertical cursor movements

PR-URL: #58003
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>

Author: puskin

lib/internal/readline/interface.js

@@ -155,6 +155,8 @@ const kSavePreviousState = Symbol('_savePreviousState');
 const kRestorePreviousState = Symbol('_restorePreviousState');
 const kPreviousLine = Symbol('_previousLine');
 const kPreviousCursor = Symbol('_previousCursor');
+const kPreviousCursorCols = Symbol('_previousCursorCols');
+const kMultilineMove = Symbol('_multilineMove');
 const kPreviousPrevRows = Symbol('_previousPrevRows');
 const kAddNewLineOnTTY = Symbol('_addNewLineOnTTY');
 
@@ -245,6 +247,7 @@ function InterfaceConstructor(input, output, completer, terminal) {
   this[kRedoStack] = [];
   this.history = history;
   this.historySize = historySize;
+  this[kPreviousCursorCols] = -1;
 
   // The kill ring is a global list of blocks of text that were previously
   // killed (deleted). If its size exceeds kMaxLengthOfKillRing, the oldest
@@ -1114,27 +1117,50 @@ class Interface extends InterfaceConstructor {
     this[kRefreshLine]();
   }
 
-  [kMoveDownOrHistoryNext]() {
-    const { cols, rows } = this.getCursorPos();
-    const splitLine = StringPrototypeSplit(this.line, '\n');
-    if (!this.historyIndex && rows === splitLine.length) {
-      return;
+  [kMultilineMove](direction, splitLines, { rows, cols }) {
+    const curr = splitLines[rows];
+    const down = direction === 1;
+    const adj = splitLines[rows + direction];
+    const promptLen = kMultilinePrompt.description.length;
+    let amountToMove;
+    // Clamp distance to end of current + prompt + next/prev line + newline
+    const clamp = down ?
+      curr.length - cols + promptLen + adj.length + 1 :
+      -cols + 1;
+    const shouldClamp = cols > adj.length + 1;
+
+    if (shouldClamp) {
+      if (this[kPreviousCursorCols] === -1) {
+        this[kPreviousCursorCols] = cols;
+      }
+      amountToMove = clamp;
+    } else {
+      if (down) {
+        amountToMove = curr.length + 1;
+      } else {
+        amountToMove = -adj.length - 1;
+      }
+      if (this[kPreviousCursorCols] !== -1) {
+        if (this[kPreviousCursorCols] <= adj.length) {
+          amountToMove += this[kPreviousCursorCols] - cols;
+          this[kPreviousCursorCols] = -1;
+        } else {
+          amountToMove = clamp;
+        }
+      }
     }
-    // Go to the next history only if the cursor is in the first line of the multiline input.
-    // Otherwise treat the "arrow down" as a movement to the next row.
-    if (this[kIsMultiline] && rows < splitLine.length - 1) {
-      const currentLine = splitLine[rows];
-      const nextLine = splitLine[rows + 1];
-      // If I am moving down and the current line is longer than the next line
-      const amountToMove = (cols > nextLine.length + 1) ?
-        currentLine.length - cols + nextLine.length +
-        kMultilinePrompt.description.length + 1 : // Move to the end of the current line
-        // + chars to account for the kMultilinePrompt prefix, + 1 to go to the first char
-        currentLine.length + 1; // Otherwise just move to the next line, in the same position
-      this[kMoveCursor](amountToMove);
+
+    this[kMoveCursor](amountToMove);
+  }
+
+  [kMoveDownOrHistoryNext]() {
+    const cursorPos = this.getCursorPos();
+    const splitLines = StringPrototypeSplit(this.line, '\n');
+    if (this[kIsMultiline] && cursorPos.rows < splitLines.length - 1) {
+      this[kMultilineMove](1, splitLines, cursorPos);
       return;
     }
-
+    this[kPreviousCursorCols] = -1;
     this[kHistoryNext]();
   }
 
@@ -1169,23 +1195,13 @@ class Interface extends InterfaceConstructor {
   }
 
   [kMoveUpOrHistoryPrev]() {
-    const { cols, rows } = this.getCursorPos();
-    if (this.historyIndex === this.history.length && rows) {
-      return;
-    }
-    // Go to the previous history only if the cursor is in the first line of the multiline input.
-    // Otherwise treat the "arrow up" as a movement to the previous row.
-    if (this[kIsMultiline] && rows > 0) {
-      const splitLine = StringPrototypeSplit(this.line, '\n');
-      const previousLine = splitLine[rows - 1];
-      // If I am moving up and the current line is longer than the previous line
-      const amountToMove = (cols > previousLine.length + 1) ?
-        -cols + 1 : // Move to the beginning of the current line + 1 char to go to the end of the previous line
-        -previousLine.length - 1; // Otherwise just move to the previous line, in the same position
-      this[kMoveCursor](amountToMove);
+    const cursorPos = this.getCursorPos();
+    if (this[kIsMultiline] && cursorPos.rows > 0) {
+      const splitLines = StringPrototypeSplit(this.line, '\n');
+      this[kMultilineMove](-1, splitLines, cursorPos);
       return;
     }
-
+    this[kPreviousCursorCols] = -1;
     this[kHistoryPrev]();
   }
 
@@ -1296,6 +1312,7 @@ class Interface extends InterfaceConstructor {
     const previousKey = this[kPreviousKey];
     key ||= kEmptyObject;
     this[kPreviousKey] = key;
+    let shouldResetPreviousCursorCols = true;
 
     if (!key.meta || key.name !== 'y') {
       // Reset yanking state unless we are doing yank pop.
@@ -1543,10 +1560,12 @@ class Interface extends InterfaceConstructor {
           break;
 
         case 'up':
+          shouldResetPreviousCursorCols = false;
           this[kMoveUpOrHistoryPrev]();
           break;
 
         case 'down':
+          shouldResetPreviousCursorCols = false;
           this[kMoveDownOrHistoryNext]();
           break;
 
@@ -1582,6 +1601,9 @@ class Interface extends InterfaceConstructor {
           }
       }
     }
+    if (shouldResetPreviousCursorCols) {
+      this[kPreviousCursorCols] = -1;
+    }
   }
 
   /**

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

@@ -55,7 +55,7 @@ tmpdir.refresh();
     r.write('22222222222222'); // The command is not complete yet. I can still edit it
     r.input.run([{ name: 'up' }]);
     r.input.run([{ name: 'up' }]); // I am on the first line
-    for (let i = 0; i < 4; i++) {
+    for (let i = 0; i < 3; i++) {
       r.input.run([{ name: 'right' }]);
     } // I am at the end of the first line
     assert.strictEqual(r.cursor, 17);
@@ -101,7 +101,7 @@ tmpdir.refresh();
     r.write('22222222222222'); // The command is not complete yet. I can still edit it
     r.input.run([{ name: 'up' }]);
     r.input.run([{ name: 'up' }]); // I am on the first line
-    for (let i = 0; i < 2; i++) {
+    for (let i = 0; i < 3; i++) {
       r.input.run([{ name: 'left' }]);
     } // I am right after the string definition
     assert.strictEqual(r.cursor, 11);

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

@@ -71,16 +71,77 @@ tmpdir.refresh();
     assert.strictEqual(r.cursor, 15);
     r.input.run([{ name: 'up' }]);
 
-    for (let i = 0; i < 5; i++) {
+    for (let i = 0; i < 4; i++) {
       r.input.run([{ name: 'right' }]);
     }
-    assert.strictEqual(r.cursor, 8);
+    assert.strictEqual(r.cursor, 11);
 
     r.input.run([{ name: 'down' }]);
     assert.strictEqual(r.cursor, 15);
 
     r.input.run([{ name: 'down' }]);
-    assert.strictEqual(r.cursor, 19);
+    assert.strictEqual(r.cursor, 27);
+  });
+
+  repl.createInternalRepl(
+    { NODE_REPL_HISTORY: historyPath },
+    {
+      terminal: true,
+      input: new ActionStream(),
+      output: new stream.Writable({
+        write(chunk, _, next) {
+          next();
+        }
+      }),
+    },
+    checkResults
+  );
+}
+
+{
+  const historyPath = tmpdir.resolve(`.${Math.floor(Math.random() * 10000)}`);
+  // Make sure the cursor is at the right places.
+  // This is testing cursor clamping and restoring when moving up and down from long lines.
+  const checkResults = common.mustSucceed((r) => {
+    r.write('let ddd = `000');
+    r.input.run([{ name: 'enter' }]);
+    r.write('1111111111111');
+    r.input.run([{ name: 'enter' }]);
+    r.write('22222');
+    r.input.run([{ name: 'enter' }]);
+    r.write('2222');
+    r.input.run([{ name: 'enter' }]);
+    r.write('22222');
+    r.input.run([{ name: 'enter' }]);
+    r.write('33333333`');
+    r.input.run([{ name: 'up' }]);
+    assert.strictEqual(r.cursor, 45);
+
+    r.input.run([{ name: 'up' }]);
+    assert.strictEqual(r.cursor, 39);
+
+    r.input.run([{ name: 'up' }]);
+    assert.strictEqual(r.cursor, 34);
+
+    r.input.run([{ name: 'up' }]);
+    assert.strictEqual(r.cursor, 24);
+
+    r.input.run([{ name: 'right' }]);
+    // This is to reach a cursor pos which is much higher than the line we want to go to,
+    // So we can check that the cursor is clamped to the end of the line.
+    r.input.run([{ name: 'right' }]);
+
+    r.input.run([{ name: 'down' }]);
+    assert.strictEqual(r.cursor, 34);
+
+    r.input.run([{ name: 'down' }]);
+    assert.strictEqual(r.cursor, 39);
+
+    r.input.run([{ name: 'down' }]);
+    assert.strictEqual(r.cursor, 45);
+
+    r.input.run([{ name: 'down' }]);
+    assert.strictEqual(r.cursor, 55);
   });
 
   repl.createInternalRepl(