From 05c19bdbf0d9c4d78725749320655c363d362b5a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 26 Dec 2019 21:00:38 +0100
Subject: [PATCH 01/14] repl: improve preview length calculation

The preview had an off by one error in case colors where deactivated
and did not take fullwidth unicode characters into account when
displaying the preview.
---
 lib/internal/repl/utils.js                    | 56 +++++++++++++------
 test/parallel/test-repl-history-navigation.js |  4 +-
 2 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
index 4da23e096c95ed..6eee8d3c273ffa 100644
--- a/lib/internal/repl/utils.js
+++ b/lib/internal/repl/utils.js
@@ -31,26 +31,19 @@ const {
 } = require('readline');
 
 const {
-  commonPrefix
+  commonPrefix,
+  getStringWidth,
 } = require('internal/readline/utils');
 
 const { inspect } = require('util');
 
 const debug = require('internal/util/debuglog').debuglog('repl');
 
-const inspectOptions = {
-  depth: 1,
+const previewOptions = {
   colors: false,
-  compact: true,
-  breakLength: Infinity
-};
-// Specify options that might change the output in a way that it's not a valid
-// stringified object anymore.
-const inspectedOptions = inspect(inspectOptions, {
   depth: 1,
-  colors: false,
   showHidden: false
-});
+};
 
 // If the error is that we've unexpectedly ended the input,
 // then let the user try to recover by adding more input.
@@ -254,7 +247,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         }
         const { result } = preview;
         if (result.value !== undefined) {
-          callback(null, inspect(result.value, inspectOptions));
+          callback(null, inspect(result.value, previewOptions));
         // Ignore EvalErrors, SyntaxErrors and ReferenceErrors. It is not clear
         // where they came from and if they are recoverable or not. Other errors
         // may be inspected.
@@ -264,8 +257,19 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
                      result.className === 'ReferenceError')) {
           callback(null, null);
         } else if (result.objectId) {
+          // The writer options might change and have influence on the inspect
+          // output. The user might change e.g., `showProxy`, `getters` or
+          // `showHidden`. Use `inspect` instead of `JSON.stringify` to keep
+          // `Infinity` and similar intact.
+          const inspectOptions = inspect({
+            ...repl.writer.options,
+            colors: false,
+            depth: 1,
+            compact: true,
+            breakLength: Infinity
+          }, previewOptions);
           session.post('Runtime.callFunctionOn', {
-            functionDeclaration: `(v) => util.inspect(v, ${inspectedOptions})`,
+            functionDeclaration: `(v) => util.inspect(v, ${inspectOptions})`,
             objectId: result.objectId,
             arguments: [result]
           }, (error, preview) => {
@@ -337,15 +341,33 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
       // Limit the output to maximum 250 characters. Otherwise it becomes a)
       // difficult to read and b) non terminal REPLs would visualize the whole
       // output.
-      const maxColumns = MathMin(repl.columns, 250);
-
-      if (inspected.length > maxColumns) {
-        inspected = `${inspected.slice(0, maxColumns - 6)}...`;
+      let maxColumns = MathMin(repl.columns, 250);
+
+      // Support unicode characters of width other than one by checking the
+      // actual width.
+      // TODO(BridgeAR): Add a test case to verify full-width characters work as
+      // expected. Also test that the line break in case of deactivated colors
+      // work as expected.
+      if (inspected.length * 2 >= maxColumns &&
+          getStringWidth(inspected) > maxColumns) {
+        maxColumns -= 4 + (repl.useColors ? 0 : 3);
+        let res = '';
+        for (const char of inspected) {
+          maxColumns -= getStringWidth(char);
+          if (maxColumns < 0)
+            break;
+          res += char;
+        }
+        inspected = `${res}...`;
       }
+
+      // Line breaks are very rare and probably only occur in case of error
+      // messages with line breaks.
       const lineBreakPos = inspected.indexOf('\n');
       if (lineBreakPos !== -1) {
         inspected = `${inspected.slice(0, lineBreakPos)}`;
       }
+
       const result = repl.useColors ?
         `\u001b[90m${inspected}\u001b[39m` :
         `// ${inspected}`;
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index 8de2b49b0ea5a6..8dcdf45cf9024d 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -85,7 +85,7 @@ const tests = [
                  '144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
                  ' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
                  '1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
-                 ' 2025, 2116, 2209, ...',
+                 ' 2025, 2116, 2209,...',
                `${prompt}{key : {key2 :[] }}`,
                prev && '\n// { key: { key2: [] } }',
                `${prompt}555 + 909`,
@@ -100,7 +100,7 @@ const tests = [
                  '144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
                  ' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
                  '1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
-                 ' 2025, 2116, 2209, ...',
+                 ' 2025, 2116, 2209,...',
                prompt].filter((e) => typeof e === 'string'),
     clean: true
   },

From 470e87fbd8e638e1418e30d565167bb70af12d40 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 27 Dec 2019 15:32:39 +0100
Subject: [PATCH 02/14] readline,repl: add substring based history search

This improves the current history search feature by adding substring
based history search similar to ZSH. In case the `UP` or `DOWN`
buttons are pressed after writing a few characters, the start string
up to the current cursor is used to search the history.

All other history features work exactly as they used to.

Fixes: https://github.com/nodejs/node/issues/28437
---
 doc/api/repl.md                               |  9 ++-
 lib/internal/readline/utils.js                |  3 +
 lib/internal/repl/utils.js                    |  2 +
 lib/readline.js                               | 65 +++++++++++++----
 test/parallel/test-readline-interface.js      | 36 ++++++++-
 test/parallel/test-repl-history-navigation.js | 73 +++++++++++++++++--
 test/parallel/test-repl-reverse-search.js     |  8 +-
 7 files changed, 163 insertions(+), 33 deletions(-)

diff --git a/doc/api/repl.md b/doc/api/repl.md
index a030da7b4df460..48650e40d247c1 100644
--- a/doc/api/repl.md
+++ b/doc/api/repl.md
@@ -22,10 +22,11 @@ be connected to any Node.js [stream][].
 
 Instances of [`repl.REPLServer`][] support automatic completion of inputs,
 completion preview, simplistic Emacs-style line editing, multi-line inputs,
-[ZSH][] like reverse-i-search, ANSI-styled output, saving and restoring current
-REPL session state, error recovery, and customizable evaluation functions.
-Terminals that do not support ANSI-styles and Emacs-style line editing
-automatically fall back to a limited feature set.
+[ZSH][]-like reverse-i-search, [ZSH][]-like substring-based history search,
+ANSI-styled output, saving and restoring current REPL session state, error
+recovery, and customizable evaluation functions. Terminals that do not support
+ANSI styles and Emacs-style line editing automatically fall back to a limited
+feature set.
 
 ### Commands and Special Keys
 
diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index 3ff50124e74e70..6a2d1553655071 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -4,6 +4,7 @@ const {
   Boolean,
   NumberIsInteger,
   RegExp,
+  Symbol,
 } = primordials;
 
 // Regex used for ansi escape code splitting
@@ -17,6 +18,7 @@ const ansi = new RegExp(ansiPattern, 'g');
 
 const kUTF16SurrogateThreshold = 0x10000; // 2 ** 16
 const kEscape = '\x1b';
+const kSubstringSearch = Symbol('kSubstringSearch');
 
 let getStringWidth;
 let isFullWidthCodePoint;
@@ -470,6 +472,7 @@ module.exports = {
   emitKeys,
   getStringWidth,
   isFullWidthCodePoint,
+  kSubstringSearch,
   kUTF16SurrogateThreshold,
   stripVTControlCharacters,
   CSI
diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
index 6eee8d3c273ffa..1b9a0209f898cb 100644
--- a/lib/internal/repl/utils.js
+++ b/lib/internal/repl/utils.js
@@ -33,6 +33,7 @@ const {
 const {
   commonPrefix,
   getStringWidth,
+  kSubstringSearch,
 } = require('internal/readline/utils');
 
 const { inspect } = require('util');
@@ -646,6 +647,7 @@ function setupReverseSearch(repl) {
                typeof string !== 'string' ||
                string === '') {
       reset();
+      repl[kSubstringSearch] = '';
     } else {
       reset(`${input}${string}`);
       search();
diff --git a/lib/readline.js b/lib/readline.js
index 255fbe8754ae62..e0bdb38eb81d96 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -54,6 +54,7 @@ const {
   emitKeys,
   getStringWidth,
   isFullWidthCodePoint,
+  kSubstringSearch,
   kUTF16SurrogateThreshold,
   stripVTControlCharacters
 } = require('internal/readline/utils');
@@ -153,6 +154,7 @@ function Interface(input, output, completer, terminal) {
 
   const self = this;
 
+  this[kSubstringSearch] = null;
   this.output = output;
   this.input = input;
   this.historySize = historySize;
@@ -688,34 +690,51 @@ Interface.prototype._line = function() {
   this._onLine(line);
 };
 
-
+// TODO(BridgeAR): Add underscores to the search part and a red background in
+// case no match is found. This should only be the visual part and not the
+// actual line content!
+// TODO(BridgeAR): In case the substring based search is active and the end is
+// reached, show a comment how to search the history as before. E.g., using
+// <ctrl> + N. Only show this after two/three UPs or DOWNs, not on the first
+// one.
 Interface.prototype._historyNext = function() {
-  if (this.historyIndex > 0) {
-    this.historyIndex--;
-    this.line = this.history[this.historyIndex];
+  if (this.historyIndex >= 0) {
+    const search = this[kSubstringSearch] || '';
+    let index = this.historyIndex - 1;
+    while (index >= 0 &&
+           !this.history[index].startsWith(search)) {
+      index--;
+    }
+    if (index === -1) {
+      this.line = search;
+    } else {
+      this.line = this.history[index];
+    }
+    this.historyIndex = index;
     this.cursor = this.line.length; // Set cursor to end of line.
     this._refreshLine();
-
-  } else if (this.historyIndex === 0) {
-    this.historyIndex = -1;
-    this.cursor = 0;
-    this.line = '';
-    this._refreshLine();
   }
 };
 
-
 Interface.prototype._historyPrev = function() {
-  if (this.historyIndex + 1 < this.history.length) {
-    this.historyIndex++;
-    this.line = this.history[this.historyIndex];
+  if (this.historyIndex < this.history.length) {
+    const search = this[kSubstringSearch] || '';
+    let index = this.historyIndex + 1;
+    while (index < this.history.length &&
+           !this.history[index].startsWith(search)) {
+      index++;
+    }
+    if (index === this.history.length) {
+      return;
+    } else {
+      this.line = this.history[index];
+    }
+    this.historyIndex = index;
     this.cursor = this.line.length; // Set cursor to end of line.
-
     this._refreshLine();
   }
 };
 
-
 // Returns the last character's display position of the given string
 Interface.prototype._getDisplayPos = function(str) {
   let offset = 0;
@@ -856,6 +875,20 @@ Interface.prototype._ttyWrite = function(s, key) {
   key = key || {};
   this._previousKey = key;
 
+  // Activate or deactivate substring search.
+  if ((key.name === 'up' || key.name === 'down') &&
+      !key.ctrl && !key.meta && !key.shift) {
+    if (this[kSubstringSearch] === null) {
+      this[kSubstringSearch] = this.line.slice(0, this.cursor);
+    }
+  } else if (this[kSubstringSearch] !== null) {
+    this[kSubstringSearch] = null;
+    // Reset the index in case there's no match.
+    if (this.history.length === this.historyIndex) {
+      this.historyIndex = -1;
+    }
+  }
+
   // Ignore escape key, fixes
   // https://github.com/nodejs/node-v0.x-archive/issues/2876.
   if (key.name === 'escape') return;
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
index 3504a68ab9754a..fc189c26899886 100644
--- a/test/parallel/test-readline-interface.js
+++ b/test/parallel/test-readline-interface.js
@@ -430,6 +430,7 @@ function isWarned(emitter) {
       removeHistoryDuplicates: true
     });
     const expectedLines = ['foo', 'bar', 'baz', 'bar', 'bat', 'bat'];
+    // ['foo', 'baz', 'bar', bat'];
     let callCount = 0;
     rli.on('line', function(line) {
       assert.strictEqual(line, expectedLines[callCount]);
@@ -450,12 +451,43 @@ function isWarned(emitter) {
     assert.strictEqual(callCount, 0);
     fi.emit('keypress', '.', { name: 'down' }); // 'baz'
     assert.strictEqual(rli.line, 'baz');
+    assert.strictEqual(rli.historyIndex, 2);
     fi.emit('keypress', '.', { name: 'n', ctrl: true }); // 'bar'
     assert.strictEqual(rli.line, 'bar');
+    assert.strictEqual(rli.historyIndex, 1);
+    fi.emit('keypress', '.', { name: 'n', ctrl: true });
+    assert.strictEqual(rli.line, 'bat');
+    assert.strictEqual(rli.historyIndex, 0);
+    // Activate the substring history search.
     fi.emit('keypress', '.', { name: 'down' }); // 'bat'
     assert.strictEqual(rli.line, 'bat');
-    fi.emit('keypress', '.', { name: 'down' }); // ''
-    assert.strictEqual(rli.line, '');
+    assert.strictEqual(rli.historyIndex, -1);
+    // Deactivate substring history search.
+    fi.emit('keypress', '.', { name: 'backspace' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, -1);
+    assert.strictEqual(rli.line, 'ba');
+    // Activate the substring history search.
+    fi.emit('keypress', '.', { name: 'down' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, -1);
+    assert.strictEqual(rli.line, 'ba');
+    fi.emit('keypress', '.', { name: 'down' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, -1);
+    assert.strictEqual(rli.line, 'ba');
+    fi.emit('keypress', '.', { name: 'up' }); // 'bat'
+    assert.strictEqual(rli.historyIndex, 0);
+    assert.strictEqual(rli.line, 'bat');
+    fi.emit('keypress', '.', { name: 'up' }); // 'bar'
+    assert.strictEqual(rli.historyIndex, 1);
+    assert.strictEqual(rli.line, 'bar');
+    fi.emit('keypress', '.', { name: 'up' }); // 'baz'
+    assert.strictEqual(rli.historyIndex, 2);
+    assert.strictEqual(rli.line, 'baz');
+    fi.emit('keypress', '.', { name: 'up' }); // 'baz'
+    assert.strictEqual(rli.historyIndex, 2);
+    assert.strictEqual(rli.line, 'baz');
+    fi.emit('keypress', '.', { name: 'up' }); // 'baz'
+    assert.strictEqual(rli.historyIndex, 2);
+    assert.strictEqual(rli.line, 'baz');
     rli.close();
   }
 
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index 8dcdf45cf9024d..19a77afee0dd6c 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -78,6 +78,7 @@ const tests = [
   },
   {
     env: { NODE_REPL_HISTORY: defaultHistoryPath },
+    checkTotal: true,
     test: [UP, UP, UP, UP, UP, DOWN, DOWN, DOWN, DOWN],
     expected: [prompt,
                `${prompt}Array(100).fill(1).map((e, i) => i ** 2)`,
@@ -102,6 +103,52 @@ const tests = [
                  '1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
                  ' 2025, 2116, 2209,...',
                prompt].filter((e) => typeof e === 'string'),
+    clean: false
+  },
+  { // Creates more history entries to navigate through.
+    env: { NODE_REPL_HISTORY: defaultHistoryPath },
+    test: [
+      '555 + 909', ENTER, // Add a duplicate to the history set.
+      'const foo = true', ENTER,
+      '555n + 111n', ENTER,
+      '5 + 5', ENTER,
+      '55 - 13 === 42', ENTER
+    ],
+    expected: [],
+    clean: false
+  },
+  {
+    env: { NODE_REPL_HISTORY: defaultHistoryPath },
+    checkTotal: true,
+    preview: false,
+    showEscapeCodes: true,
+    test: [
+      '55', UP, UP, UP, UP, UP, UP, ENTER
+    ],
+    expected: [
+      '\x1B[1G', '\x1B[0J', prompt, '\x1B[3G',
+      // '55'
+      '5', '5',
+      // UP
+      '\x1B[1G', '\x1B[0J',
+      '> 55 - 13 === 42', '\x1B[17G',
+      // UP - skipping 5 + 5
+      '\x1B[1G', '\x1B[0J',
+      '> 555n + 111n', '\x1B[14G',
+      // UP - skipping const foo = true
+      '\x1B[1G', '\x1B[0J',
+      '> 555 + 909', '\x1B[12G',
+      // UP - matching the identical history entry again.
+      '\x1B[1G', '\x1B[0J',
+      '> 555 + 909',
+      // UP, UP, ENTER. UPs at the end of the history have no effect.
+      '\x1B[12G',
+      '\r\n',
+      '1464\n',
+      '\x1B[1G', '\x1B[0J',
+      '> ', '\x1B[3G',
+      '\r\n'
+    ],
     clean: true
   },
   {
@@ -190,7 +237,7 @@ const tests = [
       '\x1B[1B', '\x1B[2K', '\x1B[1A',
       // 6. Backspace
       '\x1B[1G', '\x1B[0J',
-      prompt, '\x1B[3G'
+      prompt, '\x1B[3G', '\r\n'
     ],
     clean: true
   },
@@ -259,6 +306,11 @@ const tests = [
       // 10. Word right. Cleanup
       '\x1B[0K', '\x1B[3G', '\x1B[7C', ' // n', '\x1B[10G',
       '\x1B[0K',
+      // 11. ENTER
+      '\r\n',
+      'Uncaught ReferenceError: functio is not defined\n',
+      '\x1B[1G', '\x1B[0J',
+      prompt, '\x1B[3G', '\r\n'
     ],
     clean: true
   },
@@ -300,6 +352,7 @@ const tests = [
       prompt,
       's',
       ' // Always visible',
+      prompt,
     ],
     clean: true
   }
@@ -330,8 +383,8 @@ function runTest() {
     setImmediate(runTestWrap, true);
     return;
   }
-
   const lastChunks = [];
+  let i = 0;
 
   REPL.createInternalRepl(opts.env, {
     input: new ActionStream(),
@@ -344,11 +397,11 @@ function runTest() {
           return next();
         }
 
-        lastChunks.push(inspect(output));
+        lastChunks.push(output);
 
-        if (expected.length) {
+        if (expected.length && !opts.checkTotal) {
           try {
-            assert.strictEqual(output, expected[0]);
+            assert.strictEqual(output, expected[i]);
           } catch (e) {
             console.error(`Failed test # ${numtests - tests.length}`);
             console.error('Last outputs: ' + inspect(lastChunks, {
@@ -356,7 +409,8 @@ function runTest() {
             }));
             throw e;
           }
-          expected.shift();
+          // TODO(BridgeAR): Auto close on last chunk!
+          i++;
         }
 
         next();
@@ -365,6 +419,7 @@ function runTest() {
     completer: opts.completer,
     prompt,
     useColors: false,
+    preview: opts.preview,
     terminal: true
   }, function(err, repl) {
     if (err) {
@@ -376,9 +431,13 @@ function runTest() {
       if (opts.clean)
         cleanupTmpFile();
 
-      if (expected.length !== 0) {
+      if (opts.checkTotal) {
+        assert.deepStrictEqual(lastChunks, expected);
+      } else if (expected.length !== i) {
+        console.error(tests[numtests - tests.length - 1]);
         throw new Error(`Failed test # ${numtests - tests.length}`);
       }
+
       setImmediate(runTestWrap, true);
     });
 
diff --git a/test/parallel/test-repl-reverse-search.js b/test/parallel/test-repl-reverse-search.js
index baafdc9d8fbd33..63cdcc82729597 100644
--- a/test/parallel/test-repl-reverse-search.js
+++ b/test/parallel/test-repl-reverse-search.js
@@ -309,10 +309,9 @@ function runTest() {
 
         lastChunks.push(output);
 
-        if (expected.length) {
+        if (expected.length && !opts.checkTotal) {
           try {
-            if (!opts.checkTotal)
-              assert.strictEqual(output, expected[i]);
+            assert.strictEqual(output, expected[i]);
           } catch (e) {
             console.error(`Failed test # ${numtests - tests.length}`);
             console.error('Last outputs: ' + inspect(lastChunks, {
@@ -342,7 +341,8 @@ function runTest() {
 
       if (opts.checkTotal) {
         assert.deepStrictEqual(lastChunks, expected);
-      } else if (expected.length !== 0) {
+      } else if (expected.length !== i) {
+        console.error(tests[numtests - tests.length - 1]);
         throw new Error(`Failed test # ${numtests - tests.length}`);
       }
 

From 6efc2b9ca5c2d9c89f98855339a8acb215236649 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Fri, 27 Dec 2019 19:33:49 +0100
Subject: [PATCH 03/14] readline,repl: skip history entries identical to the
 current line

Skip history entries that are identical to the currently visible line
to improve the user experience.
---
 lib/readline.js                               | 8 ++++++--
 test/parallel/test-repl-history-navigation.js | 4 ----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/readline.js b/lib/readline.js
index e0bdb38eb81d96..b7b2651aec35d9 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -702,7 +702,8 @@ Interface.prototype._historyNext = function() {
     const search = this[kSubstringSearch] || '';
     let index = this.historyIndex - 1;
     while (index >= 0 &&
-           !this.history[index].startsWith(search)) {
+           (!this.history[index].startsWith(search) ||
+             this.line === this.history[index])) {
       index--;
     }
     if (index === -1) {
@@ -721,10 +722,13 @@ Interface.prototype._historyPrev = function() {
     const search = this[kSubstringSearch] || '';
     let index = this.historyIndex + 1;
     while (index < this.history.length &&
-           !this.history[index].startsWith(search)) {
+           (!this.history[index].startsWith(search) ||
+             this.line === this.history[index])) {
       index++;
     }
     if (index === this.history.length) {
+      // TODO(BridgeAR): Change this to:
+      // this.line = search;
       return;
     } else {
       this.line = this.history[index];
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index 19a77afee0dd6c..6b0813e1fae36e 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -138,11 +138,7 @@ const tests = [
       // UP - skipping const foo = true
       '\x1B[1G', '\x1B[0J',
       '> 555 + 909', '\x1B[12G',
-      // UP - matching the identical history entry again.
-      '\x1B[1G', '\x1B[0J',
-      '> 555 + 909',
       // UP, UP, ENTER. UPs at the end of the history have no effect.
-      '\x1B[12G',
       '\r\n',
       '1464\n',
       '\x1B[1G', '\x1B[0J',

From 5a213fe47083b543a803242a51d8da013601918f Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sun, 29 Dec 2019 13:09:45 +0100
Subject: [PATCH 04/14] readline,repl: improve history up/previous

Reaching the history end caused the last entry to be persistent.
That way there's no actualy feedback to the user that the history
end is reached. Instead, visualize the original input line and keep
the history index at the history end in case the user wants to go
back again.
---
 lib/readline.js                               |  6 +--
 test/parallel/test-readline-interface.js      | 20 ++++++---
 test/parallel/test-repl-history-navigation.js | 14 ++++---
 test/parallel/test-repl-persistent-history.js | 41 +++++++++++++++----
 .../test-repl-programmatic-history.js         | 37 ++++++++++++++---
 5 files changed, 90 insertions(+), 28 deletions(-)

diff --git a/lib/readline.js b/lib/readline.js
index b7b2651aec35d9..5f3aeedd4e531b 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -718,7 +718,7 @@ Interface.prototype._historyNext = function() {
 };
 
 Interface.prototype._historyPrev = function() {
-  if (this.historyIndex < this.history.length) {
+  if (this.historyIndex < this.history.length && this.history.length) {
     const search = this[kSubstringSearch] || '';
     let index = this.historyIndex + 1;
     while (index < this.history.length &&
@@ -727,9 +727,7 @@ Interface.prototype._historyPrev = function() {
       index++;
     }
     if (index === this.history.length) {
-      // TODO(BridgeAR): Change this to:
-      // this.line = search;
-      return;
+      this.line = search;
     } else {
       this.line = this.history[index];
     }
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
index fc189c26899886..0f4345f40b903c 100644
--- a/test/parallel/test-readline-interface.js
+++ b/test/parallel/test-readline-interface.js
@@ -482,12 +482,20 @@ function isWarned(emitter) {
     fi.emit('keypress', '.', { name: 'up' }); // 'baz'
     assert.strictEqual(rli.historyIndex, 2);
     assert.strictEqual(rli.line, 'baz');
-    fi.emit('keypress', '.', { name: 'up' }); // 'baz'
-    assert.strictEqual(rli.historyIndex, 2);
-    assert.strictEqual(rli.line, 'baz');
-    fi.emit('keypress', '.', { name: 'up' }); // 'baz'
-    assert.strictEqual(rli.historyIndex, 2);
-    assert.strictEqual(rli.line, 'baz');
+    fi.emit('keypress', '.', { name: 'up' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, 4);
+    assert.strictEqual(rli.line, 'ba');
+    fi.emit('keypress', '.', { name: 'up' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, 4);
+    assert.strictEqual(rli.line, 'ba');
+    // Deactivate substring history search and reset history index.
+    fi.emit('keypress', '.', { name: 'right' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, -1);
+    assert.strictEqual(rli.line, 'ba');
+    // Substring history search activated.
+    fi.emit('keypress', '.', { name: 'up' }); // 'ba'
+    assert.strictEqual(rli.historyIndex, 0);
+    assert.strictEqual(rli.line, 'bat');
     rli.close();
   }
 
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index 6b0813e1fae36e..b1cdc5cbdfc41e 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -78,8 +78,7 @@ const tests = [
   },
   {
     env: { NODE_REPL_HISTORY: defaultHistoryPath },
-    checkTotal: true,
-    test: [UP, UP, UP, UP, UP, DOWN, DOWN, DOWN, DOWN],
+    test: [UP, UP, UP, UP, UP, DOWN, DOWN, DOWN, DOWN, DOWN],
     expected: [prompt,
                `${prompt}Array(100).fill(1).map((e, i) => i ** 2)`,
                prev && '\n// [ 0, 1, 4, 9, 16, 25, 36, 49, 64, 81, 100, 121, ' +
@@ -92,6 +91,8 @@ const tests = [
                `${prompt}555 + 909`,
                prev && '\n// 1464',
                `${prompt}let ab = 45`,
+               prompt,
+               `${prompt}let ab = 45`,
                `${prompt}555 + 909`,
                prev && '\n// 1464',
                `${prompt}{key : {key2 :[] }}`,
@@ -138,9 +139,12 @@ const tests = [
       // UP - skipping const foo = true
       '\x1B[1G', '\x1B[0J',
       '> 555 + 909', '\x1B[12G',
-      // UP, UP, ENTER. UPs at the end of the history have no effect.
-      '\r\n',
-      '1464\n',
+      // UP, UP
+      // UPs at the end of the history reset the line to the original input.
+      '\x1B[1G', '\x1B[0J',
+      '> 55', '\x1B[5G',
+      // ENTER
+      '\r\n', '55\n',
       '\x1B[1G', '\x1B[0J',
       '> ', '\x1B[3G',
       '\r\n'
diff --git a/test/parallel/test-repl-persistent-history.js b/test/parallel/test-repl-persistent-history.js
index bb10085eccfcf6..1d1261a3752365 100644
--- a/test/parallel/test-repl-persistent-history.js
+++ b/test/parallel/test-repl-persistent-history.js
@@ -10,6 +10,7 @@ const assert = require('assert');
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
+const util = require('util');
 
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
@@ -51,6 +52,7 @@ ActionStream.prototype.readable = true;
 
 // Mock keys
 const UP = { name: 'up' };
+const DOWN = { name: 'down' };
 const ENTER = { name: 'enter' };
 const CLEAR = { ctrl: true, name: 'u' };
 
@@ -90,20 +92,42 @@ const tests = [
   },
   {
     env: {},
-    test: [UP, '\'42\'', ENTER],
-    expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
+    test: [UP, '21', ENTER, "'42'", ENTER],
+    expected: [
+      prompt,
+      '2', '1', '21\n', prompt, prompt,
+      "'", '4', '2', "'", "'42'\n", prompt, prompt
+    ],
     clean: false
   },
   { // Requires the above test case
     env: {},
-    test: [UP, UP, ENTER],
-    expected: [prompt, `${prompt}'42'`, '\'42\'\n', prompt]
+    test: [UP, UP, CLEAR, ENTER, DOWN, CLEAR, ENTER, UP, ENTER],
+    expected: [
+      prompt,
+      `${prompt}'42'`,
+      `${prompt}21`,
+      prompt,
+      prompt,
+      `${prompt}'42'`,
+      prompt,
+      prompt,
+      `${prompt}21`,
+      '21\n',
+      prompt,
+    ]
   },
   {
     env: { NODE_REPL_HISTORY: historyPath,
            NODE_REPL_HISTORY_SIZE: 1 },
-    test: [UP, UP, CLEAR],
-    expected: [prompt, `${prompt}'you look fabulous today'`, prompt]
+    test: [UP, UP, DOWN, CLEAR],
+    expected: [
+      prompt,
+      `${prompt}'you look fabulous today'`,
+      prompt,
+      `${prompt}'you look fabulous today'`,
+      prompt
+    ]
   },
   {
     env: { NODE_REPL_HISTORY: historyPathFail,
@@ -169,6 +193,8 @@ function runTest(assertCleaned) {
   const opts = tests.shift();
   if (!opts) return; // All done
 
+  console.log('NEW');
+
   if (assertCleaned) {
     try {
       assert.strictEqual(fs.readFileSync(defaultHistoryPath, 'utf8'), '');
@@ -193,6 +219,7 @@ function runTest(assertCleaned) {
     output: new stream.Writable({
       write(chunk, _, next) {
         const output = chunk.toString();
+        console.log('INPUT', util.inspect(output));
 
         // Ignore escapes and blank lines
         if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output))
@@ -207,7 +234,7 @@ function runTest(assertCleaned) {
         next();
       }
     }),
-    prompt: prompt,
+    prompt,
     useColors: false,
     terminal: true
   }, function(err, repl) {
diff --git a/test/parallel/test-repl-programmatic-history.js b/test/parallel/test-repl-programmatic-history.js
index 7eda401a7c386b..5307ae0556ae74 100644
--- a/test/parallel/test-repl-programmatic-history.js
+++ b/test/parallel/test-repl-programmatic-history.js
@@ -8,6 +8,7 @@ const assert = require('assert');
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
+const util = require('util');
 
 const tmpdir = require('../common/tmpdir');
 tmpdir.refresh();
@@ -49,6 +50,7 @@ ActionStream.prototype.readable = true;
 
 // Mock keys
 const UP = { name: 'up' };
+const DOWN = { name: 'down' };
 const ENTER = { name: 'enter' };
 const CLEAR = { ctrl: true, name: 'u' };
 
@@ -88,20 +90,40 @@ const tests = [
   },
   {
     env: {},
-    test: [UP, '\'42\'', ENTER],
-    expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
+    test: [UP, '21', ENTER, "'42'", ENTER],
+    expected: [
+      prompt,
+      // TODO(BridgeAR): The line is refreshed too many times. The double prompt
+      // is redundant and can be optimized away.
+      '2', '1', '21\n', prompt, prompt,
+      "'", '4', '2', "'", "'42'\n", prompt, prompt
+    ],
     clean: false
   },
   { // Requires the above test case
     env: {},
-    test: [UP, UP, ENTER],
-    expected: [prompt, `${prompt}'42'`, '\'42\'\n', prompt]
+    test: [UP, UP, UP, DOWN, ENTER],
+    expected: [
+      prompt,
+      `${prompt}'42'`,
+      `${prompt}21`,
+      prompt,
+      `${prompt}21`,
+      '21\n',
+      prompt
+    ]
   },
   {
     env: { NODE_REPL_HISTORY: historyPath,
            NODE_REPL_HISTORY_SIZE: 1 },
-    test: [UP, UP, CLEAR],
-    expected: [prompt, `${prompt}'you look fabulous today'`, prompt]
+    test: [UP, UP, DOWN, CLEAR],
+    expected: [
+      prompt,
+      `${prompt}'you look fabulous today'`,
+      prompt,
+      `${prompt}'you look fabulous today'`,
+      prompt
+    ]
   },
   {
     env: { NODE_REPL_HISTORY: historyPathFail,
@@ -167,6 +189,8 @@ function runTest(assertCleaned) {
   const opts = tests.shift();
   if (!opts) return; // All done
 
+  console.log('NEW');
+
   if (assertCleaned) {
     try {
       assert.strictEqual(fs.readFileSync(defaultHistoryPath, 'utf8'), '');
@@ -192,6 +216,7 @@ function runTest(assertCleaned) {
     output: new stream.Writable({
       write(chunk, _, next) {
         const output = chunk.toString();
+        console.log('INPUT', util.inspect(output));
 
         // Ignore escapes and blank lines
         if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output))

From 3b8315c27af14fd91f8f7896cd5ce8834f219e5a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 31 Dec 2019 15:07:37 +0100
Subject: [PATCH 05/14] readline: simplify getStringWidth()

Simplify getStringWidth by removing dead code (the options were
unused) and by refactoring the logic.
---
 lib/internal/readline/utils.js           | 58 ++++++------------------
 lib/readline.js                          | 16 ++-----
 test/parallel/test-readline-interface.js | 25 +++++-----
 3 files changed, 29 insertions(+), 70 deletions(-)

diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index 6a2d1553655071..9360435971792b 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -1,8 +1,6 @@
 'use strict';
 
 const {
-  Boolean,
-  NumberIsInteger,
   RegExp,
   Symbol,
 } = primordials;
@@ -21,7 +19,6 @@ const kEscape = '\x1b';
 const kSubstringSearch = Symbol('kSubstringSearch');
 
 let getStringWidth;
-let isFullWidthCodePoint;
 
 function CSI(strings, ...args) {
   let ret = `${kEscape}[`;
@@ -41,61 +38,34 @@ CSI.kClearScreenDown = CSI`0J`;
 
 if (internalBinding('config').hasIntl) {
   const icu = internalBinding('icu');
-  getStringWidth = function getStringWidth(str, options) {
-    options = options || {};
-    if (NumberIsInteger(str)) {
-      // Provide information about the character with code point 'str'.
-      return icu.getStringWidth(
-        str,
-        Boolean(options.ambiguousAsFullWidth),
-        false
-      );
-    }
-    str = stripVTControlCharacters(String(str));
+  // icu.getStringWidth(string, ambiguousAsFullWidth, expandEmojiSequence)
+  // Defaults: ambiguousAsFullWidth = false; expandEmojiSequence = true;
+  getStringWidth = function getStringWidth(str) {
     let width = 0;
+    str = stripVTControlCharacters(str);
     for (let i = 0; i < str.length; i++) {
       // Try to avoid calling into C++ by first handling the ASCII portion of
       // the string. If it is fully ASCII, we skip the C++ part.
       const code = str.charCodeAt(i);
-      if (code < 127) {
-        width += code >= 32;
-        continue;
+      if (code >= 127) {
+        width += icu.getStringWidth(str.slice(i));
+        break;
       }
-      width += icu.getStringWidth(
-        str.slice(i),
-        Boolean(options.ambiguousAsFullWidth),
-        Boolean(options.expandEmojiSequence)
-      );
-      break;
+      width += code >= 32 ? 1 : 0;
     }
     return width;
   };
-  isFullWidthCodePoint =
-    function isFullWidthCodePoint(code, options) {
-      if (typeof code !== 'number')
-        return false;
-      return icu.getStringWidth(code, options) === 2;
-    };
 } else {
   /**
    * Returns the number of columns required to display the given string.
    */
   getStringWidth = function getStringWidth(str) {
-    if (NumberIsInteger(str))
-      return isFullWidthCodePoint(str) ? 2 : 1;
-
     let width = 0;
 
-    str = stripVTControlCharacters(String(str));
-
-    for (let i = 0; i < str.length; i++) {
-      const code = str.codePointAt(i);
-
-      if (code >= kUTF16SurrogateThreshold) { // Surrogates.
-        i++;
-      }
+    str = stripVTControlCharacters(str);
 
-      if (isFullWidthCodePoint(code)) {
+    for (const char of str) {
+      if (isFullWidthCodePoint(char.codePointAt(0))) {
         width += 2;
       } else {
         width++;
@@ -109,10 +79,10 @@ if (internalBinding('config').hasIntl) {
    * Returns true if the character represented by a given
    * Unicode code point is full-width. Otherwise returns false.
    */
-  isFullWidthCodePoint = function isFullWidthCodePoint(code) {
+  const isFullWidthCodePoint = (code) => {
     // Code points are derived from:
     // http://www.unicode.org/Public/UNIDATA/EastAsianWidth.txt
-    return NumberIsInteger(code) && code >= 0x1100 && (
+    return code >= 0x1100 && (
       code <= 0x115f ||  // Hangul Jamo
       code === 0x2329 || // LEFT-POINTING ANGLE BRACKET
       code === 0x232a || // RIGHT-POINTING ANGLE BRACKET
@@ -471,9 +441,7 @@ module.exports = {
   commonPrefix,
   emitKeys,
   getStringWidth,
-  isFullWidthCodePoint,
   kSubstringSearch,
-  kUTF16SurrogateThreshold,
   stripVTControlCharacters,
   CSI
 };
diff --git a/lib/readline.js b/lib/readline.js
index 5f3aeedd4e531b..1c0e402d57ba90 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -53,9 +53,7 @@ const {
   CSI,
   emitKeys,
   getStringWidth,
-  isFullWidthCodePoint,
   kSubstringSearch,
-  kUTF16SurrogateThreshold,
   stripVTControlCharacters
 } = require('internal/readline/utils');
 
@@ -743,18 +741,14 @@ Interface.prototype._getDisplayPos = function(str) {
   const col = this.columns;
   let rows = 0;
   str = stripVTControlCharacters(str);
-  for (let i = 0, len = str.length; i < len; i++) {
-    const code = str.codePointAt(i);
-    if (code >= kUTF16SurrogateThreshold) { // Surrogates.
-      i++;
-    }
-    if (code === 0x0a) { // new line \n
-      // rows must be incremented by 1 even if offset = 0 or col = +Infinity
+  for (const char of str) {
+    if (char === '\n') {
+      // Rows must be incremented by 1 even if offset = 0 or col = +Infinity.
       rows += MathCeil(offset / col) || 1;
       offset = 0;
       continue;
     }
-    const width = getStringWidth(code);
+    const width = getStringWidth(char);
     if (width === 0 || width === 1) {
       offset += width;
     } else { // width === 2
@@ -781,7 +775,7 @@ Interface.prototype.getCursorPos = function() {
   // move the cursor to the beginning of the next line.
   if (cols + 1 === columns &&
       this.cursor < this.line.length &&
-      isFullWidthCodePoint(this.line.codePointAt(this.cursor))) {
+      getStringWidth(this.line[this.cursor]) > 1) {
     rows++;
     cols = 0;
   }
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
index 0f4345f40b903c..73f743d03312e2 100644
--- a/test/parallel/test-readline-interface.js
+++ b/test/parallel/test-readline-interface.js
@@ -1159,27 +1159,24 @@ function isWarned(emitter) {
     }
   }
 
-  // isFullWidthCodePoint() should return false for non-numeric values
-  [true, false, null, undefined, {}, [], 'あ'].forEach((v) => {
-    assert.strictEqual(internalReadline.isFullWidthCodePoint('あ'), false);
-  });
-
   // Wide characters should be treated as two columns.
-  assert.strictEqual(internalReadline.isFullWidthCodePoint('a'.charCodeAt(0)),
-                     false);
-  assert.strictEqual(internalReadline.isFullWidthCodePoint('あ'.charCodeAt(0)),
-                     true);
-  assert.strictEqual(internalReadline.isFullWidthCodePoint('谢'.charCodeAt(0)),
-                     true);
-  assert.strictEqual(internalReadline.isFullWidthCodePoint('고'.charCodeAt(0)),
-                     true);
-  assert.strictEqual(internalReadline.isFullWidthCodePoint(0x1f251), true);
+  assert.strictEqual(internalReadline.getStringWidth('a'), 1);
+  assert.strictEqual(internalReadline.getStringWidth('あ'), 2);
+  assert.strictEqual(internalReadline.getStringWidth('谢'), 2);
+  assert.strictEqual(internalReadline.getStringWidth('고'), 2);
+  assert.strictEqual(
+    internalReadline.getStringWidth(String.fromCodePoint(0x1f251)), 2);
   assert.strictEqual(internalReadline.getStringWidth('abcde'), 5);
   assert.strictEqual(internalReadline.getStringWidth('古池や'), 6);
   assert.strictEqual(internalReadline.getStringWidth('ノード.js'), 9);
   assert.strictEqual(internalReadline.getStringWidth('你好'), 4);
   assert.strictEqual(internalReadline.getStringWidth('안녕하세요'), 10);
   assert.strictEqual(internalReadline.getStringWidth('A\ud83c\ude00BC'), 5);
+  assert.strictEqual(internalReadline.getStringWidth('👨‍👩‍👦‍👦'), 8);
+  assert.strictEqual(internalReadline.getStringWidth('🐕𐐷あ💻😀'), 9);
+  // TODO(BridgeAR): This should have a width of 4.
+  assert.strictEqual(internalReadline.getStringWidth('⓬⓪'), 2);
+  assert.strictEqual(internalReadline.getStringWidth('\u0301\u200D\u200E'), 0);
 
   // Check if vt control chars are stripped
   assert.strictEqual(

From d6b1bacf5dfc76c7e44a832f2e680cb7524b7bfd Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 31 Dec 2019 15:09:04 +0100
Subject: [PATCH 06/14] readline: move charLengthLeft() and charLengthAt()

This moves the charLengthLeft() and charLengthAt() into the internal
readline file. This allows sharing the functions internally with
other code.
---
 lib/internal/readline/utils.js | 25 +++++++++++++++++++++++++
 lib/readline.js                | 21 ++-------------------
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index 9360435971792b..92be75fc4ed64c 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -36,6 +36,29 @@ CSI.kClearToLineEnd = CSI`0K`;
 CSI.kClearLine = CSI`2K`;
 CSI.kClearScreenDown = CSI`0J`;
 
+// TODO(BridgeAR): Treat combined characters as single character, i.e,
+// 'a\u0301' and '\u0301a' (both have the same visual output).
+// Check Canonical_Combining_Class in
+// http://userguide.icu-project.org/strings/properties
+function charLengthLeft(str, i) {
+  if (i <= 0)
+    return 0;
+  if ((i > 1 && str.codePointAt(i - 2) >= kUTF16SurrogateThreshold) ||
+      str.codePointAt(i - 1) >= kUTF16SurrogateThreshold) {
+    return 2;
+  }
+  return 1;
+}
+
+function charLengthAt(str, i) {
+  if (str.length <= i) {
+    // Pretend to move to the right. This is necessary to autocomplete while
+    // moving to the right.
+    return 1;
+  }
+  return str.codePointAt(i) >= kUTF16SurrogateThreshold ? 2 : 1;
+}
+
 if (internalBinding('config').hasIntl) {
   const icu = internalBinding('icu');
   // icu.getStringWidth(string, ambiguousAsFullWidth, expandEmojiSequence)
@@ -438,6 +461,8 @@ function commonPrefix(strings) {
 }
 
 module.exports = {
+  charLengthAt,
+  charLengthLeft,
   commonPrefix,
   emitKeys,
   getStringWidth,
diff --git a/lib/readline.js b/lib/readline.js
index 1c0e402d57ba90..4f4d118c9f68fa 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -49,6 +49,8 @@ const { validateString } = require('internal/validators');
 const { inspect } = require('internal/util/inspect');
 const EventEmitter = require('events');
 const {
+  charLengthAt,
+  charLengthLeft,
   commonPrefix,
   CSI,
   emitKeys,
@@ -591,25 +593,6 @@ Interface.prototype._wordRight = function() {
   }
 };
 
-function charLengthLeft(str, i) {
-  if (i <= 0)
-    return 0;
-  if ((i > 1 && str.codePointAt(i - 2) >= kUTF16SurrogateThreshold) ||
-      str.codePointAt(i - 1) >= kUTF16SurrogateThreshold) {
-    return 2;
-  }
-  return 1;
-}
-
-function charLengthAt(str, i) {
-  if (str.length <= i) {
-    // Pretend to move to the right. This is necessary to autocomplete while
-    // moving to the right.
-    return 1;
-  }
-  return str.codePointAt(i) >= kUTF16SurrogateThreshold ? 2 : 1;
-}
-
 Interface.prototype._deleteLeft = function() {
   if (this.cursor > 0 && this.line.length > 0) {
     // The number of UTF-16 units comprising the character to the left

From 9f7730ef99680ad4cd9452b704324db496016a8d Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 31 Dec 2019 15:12:31 +0100
Subject: [PATCH 07/14] src: improve GetColumnWidth performance

This improves the performance in GetColumnWidth for full width
characters.
---
 src/node_i18n.cc | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/node_i18n.cc b/src/node_i18n.cc
index c68e01e1074a4a..7cb7ab35d5060c 100644
--- a/src/node_i18n.cc
+++ b/src/node_i18n.cc
@@ -728,16 +728,6 @@ static void ToASCII(const FunctionCallbackInfo<Value>& args) {
 // Refs: https://github.com/KDE/konsole/blob/8c6a5d13c0/src/konsole_wcwidth.cpp#L101-L223
 static int GetColumnWidth(UChar32 codepoint,
                           bool ambiguous_as_full_width = false) {
-  const auto zero_width_mask = U_GC_CC_MASK |  // C0/C1 control code
-                               U_GC_CF_MASK |  // Format control character
-                               U_GC_ME_MASK |  // Enclosing mark
-                               U_GC_MN_MASK;   // Nonspacing mark
-  if (codepoint != 0x00AD &&  // SOFT HYPHEN is Cf but not zero-width
-      ((U_MASK(u_charType(codepoint)) & zero_width_mask) ||
-       u_hasBinaryProperty(codepoint, UCHAR_EMOJI_MODIFIER))) {
-    return 0;
-  }
-
   // UCHAR_EAST_ASIAN_WIDTH is the Unicode property that identifies a
   // codepoint as being full width, wide, ambiguous, neutral, narrow,
   // or halfwidth.
@@ -761,6 +751,15 @@ static int GetColumnWidth(UChar32 codepoint,
     case U_EA_HALFWIDTH:
     case U_EA_NARROW:
     default:
+      const auto zero_width_mask = U_GC_CC_MASK |  // C0/C1 control code
+                                  U_GC_CF_MASK |  // Format control character
+                                  U_GC_ME_MASK |  // Enclosing mark
+                                  U_GC_MN_MASK;   // Nonspacing mark
+      if (codepoint != 0x00AD &&  // SOFT HYPHEN is Cf but not zero-width
+          ((U_MASK(u_charType(codepoint)) & zero_width_mask) ||
+          u_hasBinaryProperty(codepoint, UCHAR_EMOJI_MODIFIER))) {
+        return 0;
+      }
       return 1;
   }
 }

From e9bc9568d760780b8684e516b5938c1d5177761d Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 31 Dec 2019 15:14:04 +0100
Subject: [PATCH 08/14] src: change GetStringWidth's expand_emoji_sequence
 option default

The option is now set to true by default. Most terminals do not have
full emoji support and visualize emojis with zero width joiners as
individual emojis.
Also verify that at least one argument is always passed through to the
function and remove support for passing through code points. Only
accept strings from now on to simplify the API.
---
 lib/internal/readline/utils.js        |  3 ++
 src/node_i18n.cc                      | 13 ++---
 test/parallel/test-icu-stringwidth.js | 78 +++++++++++++--------------
 3 files changed, 43 insertions(+), 51 deletions(-)

diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index 92be75fc4ed64c..ae35c7d270e668 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -63,6 +63,9 @@ if (internalBinding('config').hasIntl) {
   const icu = internalBinding('icu');
   // icu.getStringWidth(string, ambiguousAsFullWidth, expandEmojiSequence)
   // Defaults: ambiguousAsFullWidth = false; expandEmojiSequence = true;
+  // TODO(BridgeAR): Expose the options to the user. That is probably the
+  // best thing possible at the moment, since it's difficult to know what
+  // the receiving end supports.
   getStringWidth = function getStringWidth(str) {
     let width = 0;
     str = stripVTControlCharacters(str);
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
index 7cb7ab35d5060c..46c6ef39f861ec 100644
--- a/src/node_i18n.cc
+++ b/src/node_i18n.cc
@@ -767,18 +767,10 @@ static int GetColumnWidth(UChar32 codepoint,
 // Returns the column width for the given String.
 static void GetStringWidth(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  if (args.Length() < 1)
-    return;
+  CHECK(args[0]->IsString());
 
   bool ambiguous_as_full_width = args[1]->IsTrue();
-  bool expand_emoji_sequence = args[2]->IsTrue();
-
-  if (args[0]->IsNumber()) {
-    uint32_t val;
-    if (!args[0]->Uint32Value(env->context()).To(&val)) return;
-    args.GetReturnValue().Set(GetColumnWidth(val, ambiguous_as_full_width));
-    return;
-  }
+  bool expand_emoji_sequence = !args[2]->IsBoolean() || args[2]->IsTrue();
 
   TwoByteValue value(env->isolate(), args[0]);
   // reinterpret_cast is required by windows to compile
@@ -803,6 +795,7 @@ static void GetStringWidth(const FunctionCallbackInfo<Value>& args) {
     // in advance if a particular sequence is going to be supported.
     // The expand_emoji_sequence option allows the caller to skip this
     // check and count each code within an emoji sequence separately.
+    // https://www.unicode.org/reports/tr51/tr51-16.html#Emoji_ZWJ_Sequences
     if (!expand_emoji_sequence &&
         n > 0 && p == 0x200d &&  // 0x200d == ZWJ (zero width joiner)
         (u_hasBinaryProperty(c, UCHAR_EMOJI_PRESENTATION) ||
diff --git a/test/parallel/test-icu-stringwidth.js b/test/parallel/test-icu-stringwidth.js
index 48384f916d9126..a427601803afd3 100644
--- a/test/parallel/test-icu-stringwidth.js
+++ b/test/parallel/test-icu-stringwidth.js
@@ -12,82 +12,78 @@ const readline = require('internal/readline/utils');
 
 // Ll (Lowercase Letter): LATIN SMALL LETTER A
 assert.strictEqual(readline.getStringWidth('a'), 1);
-assert.strictEqual(readline.getStringWidth(0x0061), 1);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x0061)), 1);
 // Lo (Other Letter)
 assert.strictEqual(readline.getStringWidth('丁'), 2);
-assert.strictEqual(readline.getStringWidth(0x4E01), 2);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x4E01)), 2);
 // Surrogate pairs
-assert.strictEqual(readline.getStringWidth('\ud83d\udc78\ud83c\udfff'), 2);
+assert.strictEqual(readline.getStringWidth('\ud83d\udc78\ud83c\udfff'), 4);
 assert.strictEqual(readline.getStringWidth('👅'), 2);
 // Cs (Surrogate): High Surrogate
 assert.strictEqual(readline.getStringWidth('\ud83d'), 1);
 // Cs (Surrogate): Low Surrogate
 assert.strictEqual(readline.getStringWidth('\udc78'), 1);
 // Cc (Control): NULL
-assert.strictEqual(readline.getStringWidth(0), 0);
+assert.strictEqual(readline.getStringWidth('\u0000'), 0);
 // Cc (Control): BELL
-assert.strictEqual(readline.getStringWidth(0x0007), 0);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x0007)), 0);
 // Cc (Control): LINE FEED
 assert.strictEqual(readline.getStringWidth('\n'), 0);
 // Cf (Format): SOFT HYPHEN
-assert.strictEqual(readline.getStringWidth(0x00AD), 1);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x00AD)), 1);
 // Cf (Format): LEFT-TO-RIGHT MARK
 // Cf (Format): RIGHT-TO-LEFT MARK
 assert.strictEqual(readline.getStringWidth('\u200Ef\u200F'), 1);
 // Cn (Unassigned): Not a character
-assert.strictEqual(readline.getStringWidth(0x10FFEF), 1);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x10FFEF)), 1);
 // Cn (Unassigned): Not a character (but in a CJK range)
-assert.strictEqual(readline.getStringWidth(0x3FFEF), 2);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x3FFEF)), 1);
 // Mn (Nonspacing Mark): COMBINING ACUTE ACCENT
-assert.strictEqual(readline.getStringWidth(0x0301), 0);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x0301)), 0);
 // Mc (Spacing Mark): BALINESE ADEG ADEG
 // Chosen as its Canonical_Combining_Class is not 0, but is not a 0-width
 // character.
-assert.strictEqual(readline.getStringWidth(0x1B44), 1);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x1B44)), 1);
 // Me (Enclosing Mark): COMBINING ENCLOSING CIRCLE
-assert.strictEqual(readline.getStringWidth(0x20DD), 0);
+assert.strictEqual(readline.getStringWidth(String.fromCharCode(0x20DD)), 0);
 
-// The following is an emoji sequence. In some implementations, it is
-// represented as a single glyph, in other implementations as a sequence
-// of individual glyphs. By default, the algorithm will assume the single
-// glyph interpretation and return a value of 2. By passing the
-// expandEmojiSequence: true option, each component will be counted
-// individually.
-assert.strictEqual(readline.getStringWidth('👩‍👩‍👧‍👧'), 2);
-assert.strictEqual(
-  readline.getStringWidth('👩‍👩‍👧‍👧', { expandEmojiSequence: true }), 8);
+// The following is an emoji sequence with ZWJ (zero-width-joiner). In some
+// implementations, it is represented as a single glyph, in other
+// implementations as a sequence of individual glyphs. By default, each
+// component will be counted individually, since not a lot of systems support
+// these fully.
+// See https://www.unicode.org/reports/tr51/tr51-16.html#Emoji_ZWJ_Sequences
+assert.strictEqual(readline.getStringWidth('👩‍👩‍👧‍👧'), 8);
+// TODO(BridgeAR): This should have a width of two and six. The heart contains
+// the \uFE0F variation selector that indicates that it should be displayed as
+// emoji instead of as text. Emojis are all full width characters when not being
+// rendered as text.
+// https://en.wikipedia.org/wiki/Variation_Selectors_(Unicode_block)
+assert.strictEqual(readline.getStringWidth('❤️'), 1);
+assert.strictEqual(readline.getStringWidth('👩‍❤️‍👩'), 5);
+// The length of one is correct. It is an emoji treated as text.
+assert.strictEqual(readline.getStringWidth('❤'), 1);
 
 // By default, unicode characters whose width is considered ambiguous will
 // be considered half-width. For these characters, getStringWidth will return
 // 1. In some contexts, however, it is more appropriate to consider them full
-// width. By default, the algorithm will assume half width. By passing
-// the ambiguousAsFullWidth: true option, ambiguous characters will be counted
-// as 2 columns.
+// width. By default, the algorithm will assume half width.
 assert.strictEqual(readline.getStringWidth('\u01d4'), 1);
-assert.strictEqual(
-  readline.getStringWidth('\u01d4', { ambiguousAsFullWidth: true }), 2);
 
 // Control chars and combining chars are zero
 assert.strictEqual(readline.getStringWidth('\u200E\n\u220A\u20D2'), 1);
 
 // Test that the fast path for ASCII characters yields results consistent
 // with the 'slow' path.
-for (const ambiguousAsFullWidth of [ false, true ]) {
-  for (let i = 0; i < 256; i++) {
-    const char = String.fromCharCode(i);
-    assert.strictEqual(
-      readline.getStringWidth(i, { ambiguousAsFullWidth }),
-      readline.getStringWidth(char, { ambiguousAsFullWidth }));
-    assert.strictEqual(
-      readline.getStringWidth(char + '🎉', { ambiguousAsFullWidth }),
-      readline.getStringWidth(char, { ambiguousAsFullWidth }) + 2);
+for (let i = 0; i < 256; i++) {
+  const char = String.fromCharCode(i);
+  assert.strictEqual(
+    readline.getStringWidth(char + '🎉'),
+    readline.getStringWidth(char) + 2);
 
-    if (i < 32 || (i >= 127 && i < 160)) {  // Control character
-      assert.strictEqual(
-        readline.getStringWidth(i, { ambiguousAsFullWidth }), 0);
-    } else if (i < 127) {  // Regular ASCII character
-      assert.strictEqual(
-        readline.getStringWidth(i, { ambiguousAsFullWidth }), 1);
-    }
+  if (i < 32 || (i >= 127 && i < 160)) {  // Control character
+    assert.strictEqual(readline.getStringWidth(char), 0);
+  } else {  // Regular ASCII character
+    assert.strictEqual(readline.getStringWidth(char), 1);
   }
 }

From 6dd223b506b65c8220b9180c9652edf135d39f3d Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 31 Dec 2019 15:16:09 +0100
Subject: [PATCH 09/14] readline,repl: support tabs properly

Fixes: https://github.com/nodejs/node/issues/25272
---
 lib/readline.js                    | 39 ++++++++++++++----------------
 test/parallel/test-repl-preview.js |  6 ++---
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/lib/readline.js b/lib/readline.js
index 4f4d118c9f68fa..9cbb3c55c8d951 100644
--- 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
index 4846248bdba294..1c6dfd76b89bc1 100644
--- 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']
   ];

From 26d4643b63b2f6cc9280ba5e1cbe3c83510171f0 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 4 Jan 2020 09:05:20 +0100
Subject: [PATCH 10/14] 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.
---
 lib/internal/repl/utils.js                    | 67 +++++++++++--------
 test/parallel/test-repl-history-navigation.js |  3 +
 2 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
index 1b9a0209f898cb..69910e886848bb 100644
--- 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
index b1cdc5cbdfc41e..c3710fa5a9b03d 100644
--- 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',

From 387a3279ed1b3b444c9e278e2d4935896d46b6ba Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 4 Jan 2020 11:19:57 +0100
Subject: [PATCH 11/14] test: add repl tests to verify unicode support in
 previews

This also adds a test to verify that changed writer options also
change the preview output depending on the options.
---
 lib/internal/repl/utils.js                    |   3 -
 test/parallel/test-repl-history-navigation.js | 116 +++++++++++++++++-
 2 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js
index 69910e886848bb..20904d9d6a2445 100644
--- a/lib/internal/repl/utils.js
+++ b/lib/internal/repl/utils.js
@@ -357,9 +357,6 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
 
       // Support unicode characters of width other than one by checking the
       // actual width.
-      // TODO(BridgeAR): Add a test case to verify full-width characters work as
-      // expected. Also test that the line break in case of deactivated colors
-      // work as expected.
       if (inspected.length * 2 >= maxColumns &&
           getStringWidth(inspected) > maxColumns) {
         maxColumns -= 4 + (repl.useColors ? 0 : 3);
diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js
index c3710fa5a9b03d..3591f0f336b6e6 100644
--- a/test/parallel/test-repl-history-navigation.js
+++ b/test/parallel/test-repl-history-navigation.js
@@ -59,6 +59,8 @@ const BACKSPACE = { name: 'backspace' };
 const WORD_LEFT = { name: 'left', ctrl: true };
 const WORD_RIGHT = { name: 'right', ctrl: true };
 const GO_TO_END = { name: 'end' };
+const DELETE_WORD_LEFT = { name: 'backspace', ctrl: true };
+const SIGINT = { name: 'c', ctrl: true };
 
 const prompt = '> ';
 const WAIT = '€';
@@ -155,6 +157,24 @@ const tests = [
     env: { NODE_REPL_HISTORY: defaultHistoryPath },
     skip: !process.features.inspector,
     test: [
+      // あ is a fill width character with a length of one.
+      // 🐕 is a full width character with a length of two.
+      // 𐐷 is a half width character with the length of two.
+      // '\u0301', '0x200D', '\u200E' are zero width characters.
+      `const x1 = '${'あ'.repeat(124)}'`, ENTER, // Fully visible
+      ENTER,
+      `const y1 = '${'あ'.repeat(125)}'`, ENTER, // Cut off
+      ENTER,
+      `const x2 = '${'🐕'.repeat(124)}'`, ENTER, // Fully visible
+      ENTER,
+      `const y2 = '${'🐕'.repeat(125)}'`, ENTER, // Cut off
+      ENTER,
+      `const x3 = '${'𐐷'.repeat(248)}'`, ENTER, // Fully visible
+      ENTER,
+      `const y3 = '${'𐐷'.repeat(249)}'`, ENTER, // Cut off
+      ENTER,
+      `const x4 = 'a${'\u0301'.repeat(1000)}'`, ENTER, // á
+      ENTER,
       `const ${'veryLongName'.repeat(30)} = 'I should be previewed'`,
       ENTER,
       'const e = new RangeError("visible\\ninvisible")',
@@ -174,6 +194,7 @@ const tests = [
   {
     env: { NODE_REPL_HISTORY: defaultHistoryPath },
     columns: 250,
+    checkTotal: true,
     showEscapeCodes: true,
     skip: !process.features.inspector,
     test: [
@@ -182,7 +203,21 @@ const tests = [
       UP,
       WORD_LEFT,
       UP,
-      BACKSPACE
+      BACKSPACE,
+      'x1',
+      BACKSPACE,
+      '2',
+      BACKSPACE,
+      '3',
+      BACKSPACE,
+      '4',
+      DELETE_WORD_LEFT,
+      'y1',
+      BACKSPACE,
+      '2',
+      BACKSPACE,
+      '3',
+      SIGINT
     ],
     // A = Cursor n up
     // B = Cursor n down
@@ -240,7 +275,44 @@ const tests = [
       '\x1B[1B', '\x1B[2K', '\x1B[1A',
       // 6. Backspace
       '\x1B[1G', '\x1B[0J',
-      prompt, '\x1B[3G', '\r\n'
+      '> ', '\x1B[3G', 'x', '1',
+      `\n// '${'あ'.repeat(124)}'`,
+      '\x1B[1C\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> x', '\x1B[4G', '2',
+      `\n// '${'🐕'.repeat(124)}'`,
+      '\x1B[1C\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> x', '\x1B[4G', '3',
+      `\n// '${'𐐷'.repeat(248)}'`,
+      '\x1B[1C\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> x', '\x1B[4G', '4',
+      `\n// 'a${'\u0301'.repeat(1000)}'`,
+      '\x1B[2D\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> ', '\x1B[3G', 'y', '1',
+      `\n// '${'あ'.repeat(121)}...`,
+      '\x1B[245D\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> y', '\x1B[4G', '2',
+      `\n// '${'🐕'.repeat(121)}...`,
+      '\x1B[245D\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\x1B[1G', '\x1B[0J',
+      '> y', '\x1B[4G', '3',
+      `\n// '${'𐐷'.repeat(242)}...`,
+      '\x1B[245D\x1B[1A',
+      '\x1B[1B', '\x1B[2K', '\x1B[1A',
+      '\r\n',
+      '\x1B[1G', '\x1B[0J',
+      '> ', '\x1B[3G',
+      '\r\n'
     ],
     clean: true
   },
@@ -317,6 +389,46 @@ const tests = [
     ],
     clean: true
   },
+  {
+    // Check changed inspection defaults.
+    env: { NODE_REPL_HISTORY: defaultHistoryPath },
+    skip: !process.features.inspector,
+    test: [
+      'util.inspect.replDefaults.showHidden',
+      ENTER
+    ],
+    expected: [],
+    clean: false
+  },
+  {
+    env: { NODE_REPL_HISTORY: defaultHistoryPath },
+    skip: !process.features.inspector,
+    checkTotal: true,
+    test: [
+      '[ ]',
+      WORD_LEFT,
+      WORD_LEFT,
+      UP,
+      ' = true',
+      ENTER,
+      '[ ]',
+      ENTER
+    ],
+    expected: [
+      prompt,
+      '[', ' ', ']',
+      '\n// []', '\n// []', '\n// []',
+      '> util.inspect.replDefaults.showHidden',
+      '\n// false', ' ', '\n// false',
+      '=', ' ', 't', 'r', 'u', ' // e', 'e',
+      'true\n',
+      '> ', '[', ' ', ']',
+      '\n// [ [length]: 0 ]',
+      '[ [length]: 0 ]\n',
+      '> ',
+    ],
+    clean: true
+  },
   {
     // Check that the completer ignores completions that are outdated.
     env: { NODE_REPL_HISTORY: defaultHistoryPath },

From 9f5aa5411cc72f652cafbc9276d3f312008dd474 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 4 Jan 2020 11:24:46 +0100
Subject: [PATCH 12/14] util: add todo comments for inspect to add unicode
 support

---
 lib/internal/util/inspect.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 3f2f75b909663b..21d088c5a9f98b 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -1195,6 +1195,8 @@ function groupArrayElements(ctx, output, value) {
     if (columns <= 1) {
       return output;
     }
+    // TODO(BridgeAR): Add unicode support. Use the readline getStringWidth
+    // function.
     const tmp = [];
     const maxLineLength = [];
     for (let i = 0; i < columns; i++) {
@@ -1272,6 +1274,8 @@ function formatBigInt(fn, value) {
 function formatPrimitive(fn, value, ctx) {
   if (typeof value === 'string') {
     if (ctx.compact !== true &&
+        // TODO(BridgeAR): Add unicode support. Use the readline getStringWidth
+        // function.
         value.length > kMinLineLength &&
         value.length > ctx.breakLength - ctx.indentationLvl - 4) {
       return value
@@ -1617,6 +1621,9 @@ function isBelowBreakLength(ctx, output, start, base) {
   // Each entry is separated by at least a comma. Thus, we start with a total
   // length of at least `output.length`. In addition, some cases have a
   // whitespace in-between each other that is added to the total as well.
+  // TODO(BridgeAR): Add unicode support. Use the readline getStringWidth
+  // function. Check the performance overhead and make it an opt-in in case it's
+  // significant.
   let totalLength = output.length + start;
   if (totalLength + output.length > ctx.breakLength)
     return false;

From f278c06431a80d9c9700984b26fd9d6d3226339f Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 9 Jan 2020 03:11:41 +0100
Subject: [PATCH 13/14] squash: readline: simplify getStringWidth()

readline: improve getStringWidth on non-Intl builds

The getStringWidth function returned the wrong width for multiple
inputs. It's now improved by supporting various zero width characters
and more full width characters.
---
 lib/internal/readline/utils.js           | 18 ++++++++++--
 test/parallel/test-readline-interface.js | 35 ++++--------------------
 2 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index ae35c7d270e668..6278e52c6e2613 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -91,9 +91,10 @@ if (internalBinding('config').hasIntl) {
     str = stripVTControlCharacters(str);
 
     for (const char of str) {
-      if (isFullWidthCodePoint(char.codePointAt(0))) {
+      const code = char.codePointAt(0);
+      if (isFullWidthCodePoint(code)) {
         width += 2;
-      } else {
+      } else if (!isZeroWidthCodePoint(code)) {
         width++;
       }
     }
@@ -106,7 +107,7 @@ if (internalBinding('config').hasIntl) {
    * Unicode code point is full-width. Otherwise returns false.
    */
   const isFullWidthCodePoint = (code) => {
-    // Code points are derived from:
+    // Code points are partially derived from:
     // http://www.unicode.org/Public/UNIDATA/EastAsianWidth.txt
     return code >= 0x1100 && (
       code <= 0x115f ||  // Hangul Jamo
@@ -135,10 +136,21 @@ if (internalBinding('config').hasIntl) {
       (code >= 0x1b000 && code <= 0x1b001) ||
       // Enclosed Ideographic Supplement
       (code >= 0x1f200 && code <= 0x1f251) ||
+      // Miscellaneous Symbols and Pictographs 0x1f300 - 0x1f5ff
+      // Emoticons 0x1f600 - 0x1f64f
+      (code >= 0x1f300 && code <= 0x1f64f) ||
       // CJK Unified Ideographs Extension B .. Tertiary Ideographic Plane
       (code >= 0x20000 && code <= 0x3fffd)
     );
   };
+
+  const isZeroWidthCodePoint = (code) => {
+    return code <= 0x1F || // C0 control codes
+      (code > 0x7F && code <= 0x9F) || // C1 control codes
+      (code >= 0x200B && code <= 0x200F) || // Modifying Invisible Characters
+      (code >= 0xFE00 && code <= 0xFE0F) || // Variation Selectors
+      (code >= 0xE0100 && code <= 0xE01EF); // Variation Selectors
+  };
 }
 
 /**
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
index 73f743d03312e2..cbc7ed31f488ea 100644
--- a/test/parallel/test-readline-interface.js
+++ b/test/parallel/test-readline-interface.js
@@ -731,11 +731,7 @@ function isWarned(emitter) {
       fi.emit('keypress', '.', { name: 'right' });
       cursorPos = rli.getCursorPos();
       assert.strictEqual(cursorPos.rows, 0);
-      if (common.hasIntl) {
-        assert.strictEqual(cursorPos.cols, 2);
-      } else {
-        assert.strictEqual(cursorPos.cols, 1);
-      }
+      assert.strictEqual(cursorPos.cols, 2);
 
       rli.on('line', common.mustCall((line) => {
         assert.strictEqual(line, '💻');
@@ -764,14 +760,7 @@ function isWarned(emitter) {
       fi.emit('data', '🐕');
       cursorPos = rli.getCursorPos();
       assert.strictEqual(cursorPos.rows, 0);
-
-      if (common.hasIntl) {
-        assert.strictEqual(cursorPos.cols, 2);
-      } else {
-        assert.strictEqual(cursorPos.cols, 1);
-        // Fix cursor position without internationalization
-        fi.emit('keypress', '.', { name: 'left' });
-      }
+      assert.strictEqual(cursorPos.cols, 2);
 
       rli.on('line', common.mustCall((line) => {
         assert.strictEqual(line, '🐕💻');
@@ -795,22 +784,12 @@ function isWarned(emitter) {
       fi.emit('keypress', '.', { name: 'right' });
       let cursorPos = rli.getCursorPos();
       assert.strictEqual(cursorPos.rows, 0);
-      if (common.hasIntl) {
-        assert.strictEqual(cursorPos.cols, 2);
-      } else {
-        assert.strictEqual(cursorPos.cols, 1);
-        // Fix cursor position without internationalization
-        fi.emit('keypress', '.', { name: 'right' });
-      }
+      assert.strictEqual(cursorPos.cols, 2);
 
       fi.emit('data', '🐕');
       cursorPos = rli.getCursorPos();
       assert.strictEqual(cursorPos.rows, 0);
-      if (common.hasIntl) {
-        assert.strictEqual(cursorPos.cols, 4);
-      } else {
-        assert.strictEqual(cursorPos.cols, 2);
-      }
+      assert.strictEqual(cursorPos.cols, 4);
 
       rli.on('line', common.mustCall((line) => {
         assert.strictEqual(line, '💻🐕');
@@ -972,11 +951,7 @@ function isWarned(emitter) {
       fi.emit('data', '💻');
       let cursorPos = rli.getCursorPos();
       assert.strictEqual(cursorPos.rows, 0);
-      if (common.hasIntl) {
-        assert.strictEqual(cursorPos.cols, 2);
-      } else {
-        assert.strictEqual(cursorPos.cols, 1);
-      }
+      assert.strictEqual(cursorPos.cols, 2);
       // Delete left character
       fi.emit('keypress', '.', { ctrl: true, name: 'h' });
       cursorPos = rli.getCursorPos();

From b1879aac46edbeba0a4c045ec734167d22bb8fc6 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 9 Jan 2020 07:12:33 +0100
Subject: [PATCH 14/14] fixup

---
 lib/internal/readline/utils.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
index 6278e52c6e2613..3257ee3a1e4a1f 100644
--- a/lib/internal/readline/utils.js
+++ b/lib/internal/readline/utils.js
@@ -147,8 +147,10 @@ if (internalBinding('config').hasIntl) {
   const isZeroWidthCodePoint = (code) => {
     return code <= 0x1F || // C0 control codes
       (code > 0x7F && code <= 0x9F) || // C1 control codes
+      (code >= 0x0300 && code <= 0x036F) || // Combining Diacritical Marks
       (code >= 0x200B && code <= 0x200F) || // Modifying Invisible Characters
       (code >= 0xFE00 && code <= 0xFE0F) || // Variation Selectors
+      (code >= 0xFE20 && code <= 0xFE2F) || // Combining Half Marks
       (code >= 0xE0100 && code <= 0xE01EF); // Variation Selectors
   };
 }