repl: add proper vertical cursor movements

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 kResetPreviousCursorCols = Symbol('_resetPreviousCursorCols');
 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[kResetPreviousCursorCols]();
 
   // The kill ring is a global list of blocks of text that were previously
   // killed (deleted). If its size exceeds kMaxLengthOfKillRing, the oldest
@@ -1114,6 +1117,10 @@ class Interface extends InterfaceConstructor {
     this[kRefreshLine]();
   }
 
+  [kResetPreviousCursorCols]() {
+    this[kPreviousCursorCols] = -1;
+  }
+
   [kMoveDownOrHistoryNext]() {
     const { cols, rows } = this.getCursorPos();
     const splitLine = StringPrototypeSplit(this.line, '\n');
@@ -1125,16 +1132,34 @@ class Interface extends InterfaceConstructor {
     if (this[kIsMultiline] && rows < splitLine.length - 1) {
       const currentLine = splitLine[rows];
       const nextLine = splitLine[rows + 1];
+      let amountToMove = 0;
+      const amountToClamp = currentLine.length - cols + // Go to the end of the current line
+                            kMultilinePrompt.description.length + // Add the prompt length
+                            nextLine.length + 1; // Add the length of the next line + 1 to go to the end of it
       // 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
+      const shouldClampColumn = (cols > nextLine.length + 1);
+      if (shouldClampColumn) {
+        if (this[kPreviousCursorCols] === -1) {
+          this[kPreviousCursorCols] = cols; // Save the cursor cols position to restore it later
+        }
+        amountToMove = amountToClamp;
+      } else {
+        amountToMove = currentLine.length + 1;
+        if (this[kPreviousCursorCols] !== -1) {
+          if (this[kPreviousCursorCols] <= nextLine.length) {
+            // If, while moving down, the cursor passed through a line shorter than where the cursor was
+            // I need to move it back to the original position
+            amountToMove += this[kPreviousCursorCols] - cols;
+            this[kResetPreviousCursorCols](); // Reset it to avoid moving again
+          } else {
+            amountToMove = amountToClamp; // Go to the end of next line
+          }
+        }
+      }
       this[kMoveCursor](amountToMove);
       return;
     }
-
+    this[kResetPreviousCursorCols](); // Reset it to avoid moving again
     this[kHistoryNext]();
   }
 
@@ -1178,14 +1203,38 @@ class Interface extends InterfaceConstructor {
     if (this[kIsMultiline] && rows > 0) {
       const splitLine = StringPrototypeSplit(this.line, '\n');
       const previousLine = splitLine[rows - 1];
+      let amountToMove = 0;
+      // Move to the beginning of the current line + 1 char to go to the end of the previous line
+      const amountToClamp = -cols + 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
+      const shouldClampColumn = (cols > previousLine.length + 1);
+      if (shouldClampColumn) {
+        if (this[kPreviousCursorCols] === -1) {
+          this[kPreviousCursorCols] = cols; // Save the cursor cols position to restore it later
+        }
+        amountToMove = amountToClamp;
+      } else {
+        // When pressing up a couple of times passing through shorter lines.
+        // Move to the previous line, in the same position
+        amountToMove = -previousLine.length - 1;
+        if (this[kPreviousCursorCols] !== -1) {
+          if (this[kPreviousCursorCols] <= previousLine.length) {
+            // If, while moving up, the cursor passed through a line shorter than where the cursor was
+            // I need to move it back to the original position
+            amountToMove += this[kPreviousCursorCols] - cols;
+            this[kResetPreviousCursorCols](); // Reset it to avoid moving again
+          } else {
+            amountToMove = amountToClamp; // Go to the end of the previous line
+          }
+        }
+      }
+
       this[kMoveCursor](amountToMove);
       return;
     }
 
+    this[kResetPreviousCursorCols](); // Reset it to avoid moving again
     this[kHistoryPrev]();
   }
 
@@ -1296,6 +1345,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 +1593,12 @@ class Interface extends InterfaceConstructor {
           break;
 
         case 'up':
+          shouldResetPreviousCursorCols = false;
           this[kMoveUpOrHistoryPrev]();
           break;
 
         case 'down':
+          shouldResetPreviousCursorCols = false;
           this[kMoveDownOrHistoryNext]();
           break;
 
@@ -1582,6 +1634,9 @@ class Interface extends InterfaceConstructor {
           }
       }
     }
+    if (shouldResetPreviousCursorCols) {
+      this[kResetPreviousCursorCols]();
+    }
   }
 
   /**

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(