repl: activate previews for lines exceeding the terminal columns

This improves the completion previews by activating them for lines that exceed the current terminal columns. As a drive-by fix it also simplifies some statements.
nodejs · BridgeAR · Dec 26, 2019 · Dec 27, 2019 · Dec 27, 2019 · Dec 29, 2019
commit 26d4643b63b2f6cc9280ba5e1cbe3c83510171f0
diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
@@ -136,14 +136,16 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
 
   function getPreviewPos() {
     const displayPos = repl._getDisplayPos(`${repl._prompt}${repl.line}`);
-    const cursorPos = repl.getCursorPos();
-    const rows = 1 + displayPos.rows - cursorPos.rows;
-    return { rows, cols: cursorPos.cols };
+    const cursorPos = repl.line.length !== repl.cursor ?
+      repl.getCursorPos() :
+      displayPos;
+    return { displayPos, cursorPos };
   }
 
   const clearPreview = () => {
     if (inputPreview !== null) {
-      const { rows } = getPreviewPos();
+      const { displayPos, cursorPos } = getPreviewPos();
+      const rows = displayPos.rows - cursorPos.rows + 1;
       moveCursor(repl.output, 0, rows);
       clearLine(repl.output);
       moveCursor(repl.output, 0, -rows);
@@ -153,12 +155,25 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
     if (completionPreview !== null) {
       // Prevent cursor moves if not necessary!
       const move = repl.line.length !== repl.cursor;
+      let pos, rows;
       if (move) {
-        cursorTo(repl.output, repl._prompt.length + repl.line.length);
+        pos = getPreviewPos();
+        cursorTo(repl.output, pos.displayPos.cols);
+        rows = pos.displayPos.rows - pos.cursorPos.rows;
+        moveCursor(repl.output, 0, rows);
+      }
+      const totalLine = `${repl._prompt}${repl.line}${completionPreview}`;
+      const newPos = repl._getDisplayPos(totalLine);
+      // Minimize work for the terminal. It is enough to clear the right part of
+      // the current line in case the preview is visible on a single line.
+      if (newPos.rows === 0 || (pos && pos.displayPos.rows === newPos.rows)) {
+        clearLine(repl.output, 1);
+      } else {
+        clearScreenDown(repl.output);
       }
-      clearLine(repl.output, 1);
       if (move) {
-        cursorTo(repl.output, repl._prompt.length + repl.cursor);
+        cursorTo(repl.output, pos.cursorPos.cols);
+        moveCursor(repl.output, 0, -rows);
       }
       completionPreview = null;
     }
@@ -198,17 +213,6 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
 
       const suffix = prefix.slice(completeOn.length);
 
-      const totalLength = repl.line.length +
-                          repl._prompt.length +
-                          suffix.length +
-                          (repl.useColors ? 0 : 4);
-
-      // TODO(BridgeAR): Fix me. This should not be necessary. See similar
-      // comment in `showPreview()`.
-      if (totalLength > repl.columns) {
-        return;
-      }
-
       if (insertPreview) {
         repl._insertString(suffix);
         return;
@@ -220,11 +224,17 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         `\u001b[90m${suffix}\u001b[39m` :
         ` // ${suffix}`;
 
+      const { cursorPos, displayPos } = getPreviewPos();
       if (repl.line.length !== repl.cursor) {
-        cursorTo(repl.output, repl._prompt.length + repl.line.length);
+        cursorTo(repl.output, displayPos.cols);
+        moveCursor(repl.output, 0, displayPos.rows - cursorPos.rows);
       }
       repl.output.write(result);
-      cursorTo(repl.output, repl._prompt.length + repl.cursor);
+      cursorTo(repl.output, cursorPos.cols);
+      const totalLine = `${repl._prompt}${repl.line}${suffix}`;
+      const newPos = repl._getDisplayPos(totalLine);
+      const rows = newPos.rows - cursorPos.rows - (newPos.cols === 0 ? 1 : 0);
+      moveCursor(repl.output, 0, -rows);
     });
   }
 
@@ -288,6 +298,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
     }, () => callback(new ERR_INSPECTOR_NOT_AVAILABLE()));
   }
 
+  // TODO(BridgeAR): Prevent previews while pasting code.
   const showPreview = () => {
     // Prevent duplicated previews after a refresh.
     if (inputPreview !== null) {
@@ -373,12 +384,12 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         `\u001b[90m${inspected}\u001b[39m` :
         `// ${inspected}`;
 
-      const { rows: previewRows, cols: cursorCols } = getPreviewPos();
-      if (previewRows !== 1)
-        moveCursor(repl.output, 0, previewRows - 1);
+      const { cursorPos, displayPos } = getPreviewPos();
+      const rows = displayPos.rows - cursorPos.rows;
+      moveCursor(repl.output, 0, rows);
       const { cols: resultCols } = repl._getDisplayPos(result);
       repl.output.write(`\n${result}`);
-      moveCursor(repl.output, cursorCols - resultCols, -previewRows);
+      moveCursor(repl.output, cursorPos.cols - resultCols, -rows - 1);
     });
   };
 
@@ -452,8 +463,8 @@ function setupReverseSearch(repl) {
       // Reset the already matched set in case the direction is changed. That
       // way it's possible to find those entries again.
       alreadyMatched.clear();
+      dir = keyName;
     }
-    dir = keyName;
     return true;
   }
 
@@ -598,16 +609,14 @@ function setupReverseSearch(repl) {
 
       // Clear screen and write the current repl.line before exiting.
       cursorTo(repl.output, promptPos.cols);
-      if (promptPos.rows !== 0)
-        moveCursor(repl.output, 0, promptPos.rows);
+      moveCursor(repl.output, 0, promptPos.rows);
       clearScreenDown(repl.output);
       if (repl.line !== '') {
         repl.output.write(repl.line);
         if (repl.line.length !== repl.cursor) {
           const { cols, rows } = repl.getCursorPos();
           cursorTo(repl.output, cols);
-          if (rows !== 0)
-            moveCursor(repl.output, 0, rows);
+          moveCursor(repl.output, 0, rows);
         }
       }
     }

diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
@@ -201,6 +201,9 @@ const tests = [
       // 236 + 2 + 4 + 8
       '\x1B[1G', '\x1B[0J',
       `${prompt}${' '.repeat(236)} fun`, '\x1B[243G',
+      ' // ction', '\x1B[243G',
+      ' // ction', '\x1B[243G',
+      '\x1B[0K',
       // 2. UP
       '\x1B[1G', '\x1B[0J',
       `${prompt}${' '.repeat(235)} fun`, '\x1B[242G',