readline,repl: support tabs properly

PR-URL: #31112 Fixes: #25272 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Jan 16, 2020 · 686a3bc · 686a3bc
1 parent e3491d7
commit 686a3bc
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 24 deletions.
diff --git a/lib/readline.js b/lib/readline.js
@@ -490,9 +490,6 @@ Interface.prototype._insertString = function(c) {
     } else {
       this._writeToOutput(c);
     }
-
-    // A hack to get the line refreshed if it's needed
-    this._moveCursor(0);
   }
 };
 
@@ -731,6 +728,12 @@ Interface.prototype._getDisplayPos = function(str) {
       offset = 0;
       continue;
     }
+    // Tabs must be aligned by an offset of 8.
+    // TODO(BridgeAR): Make the tab size configurable.
+    if (char === '\t') {
+      offset += 8 - (offset % 8);
+      continue;
+    }
     const width = getStringWidth(char);
     if (width === 0 || width === 1) {
       offset += width;
@@ -768,33 +771,27 @@ Interface.prototype._getCursorPos = Interface.prototype.getCursorPos;
 
 
 // This function moves cursor dx places to the right
-// (-dx for left) and refreshes the line if it is needed
+// (-dx for left) and refreshes the line if it is needed.
 Interface.prototype._moveCursor = function(dx) {
-  const oldcursor = this.cursor;
+  if (dx === 0) {
+    return;
+  }
   const oldPos = this.getCursorPos();
   this.cursor += dx;
 
-  // bounds check
-  if (this.cursor < 0) this.cursor = 0;
-  else if (this.cursor > this.line.length) this.cursor = this.line.length;
+  // Bounds check
+  if (this.cursor < 0) {
+    this.cursor = 0;
+  } else if (this.cursor > this.line.length) {
+    this.cursor = this.line.length;
+  }
 
   const newPos = this.getCursorPos();
 
-  // Check if cursors are in the same line
+  // Check if cursor stayed on the line.
   if (oldPos.rows === newPos.rows) {
-    const diffCursor = this.cursor - oldcursor;
-    let diffWidth;
-    if (diffCursor < 0) {
-      diffWidth = -getStringWidth(
-        this.line.substring(this.cursor, oldcursor)
-      );
-    } else if (diffCursor > 0) {
-      diffWidth = getStringWidth(
-        this.line.substring(this.cursor, oldcursor)
-      );
-    }
+    const diffWidth = newPos.cols - oldPos.cols;
     moveCursor(this.output, diffWidth, 0);
-    this.prevRows = newPos.rows;
   } else {
     this._refreshLine();
   }

diff --git a/test/parallel/test-repl-preview.js b/test/parallel/test-repl-preview.js
@@ -93,9 +93,9 @@ async function tests(options) {
      '\x1B[33mtrue\x1B[39m',
      '\x1B[1G\x1B[0Jrepl > \x1B[8G'],
     [' \t { a: true};', [2, 5], '\x1B[33mtrue\x1B[39m',
-     ' \t { a: tru\x1B[90me\x1B[39m\x1B[19G\x1B[0Ke}',
-     '\x1B[90m{ a: true }\x1B[39m\x1B[8C\x1B[1A\x1B[1B\x1B[2K\x1B[1A;',
-     '\x1B[90mtrue\x1B[39m\x1B[16C\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r',
+     ' \t { a: tru\x1B[90me\x1B[39m\x1B[26G\x1B[0Ke}',
+     '\x1B[90m{ a: true }\x1B[39m\x1B[16C\x1B[1A\x1B[1B\x1B[2K\x1B[1A;',
+     '\x1B[90mtrue\x1B[39m\x1B[24C\x1B[1A\x1B[1B\x1B[2K\x1B[1A\r',
      '\x1B[33mtrue\x1B[39m',
      '\x1B[1G\x1B[0Jrepl > \x1B[8G']
   ];