repl,readline: refactor common code

This renames some variables for clarity and moves the common substring part into a shared file. One algorithm was more efficient than the other but the functionality itself was identical. PR-URL: #30907 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · Dec 17, 2019 · f6f298e · f6f298e
1 parent 424c37b
commit f6f298e
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 47 deletions.
diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js
@@ -31,8 +31,8 @@ function CSI(strings, ...args) {
 }
 
 CSI.kEscape = kEscape;
-CSI.kClearToBeginning = CSI`1K`;
-CSI.kClearToEnd = CSI`0K`;
+CSI.kClearToLineBeginning = CSI`1K`;
+CSI.kClearToLineEnd = CSI`0K`;
 CSI.kClearLine = CSI`2K`;
 CSI.kClearScreenDown = CSI`0J`;
 
@@ -445,7 +445,27 @@ function* emitKeys(stream) {
   }
 }
 
+// This runs in O(n log n).
+function commonPrefix(strings) {
+  if (!strings || strings.length === 0) {
+    return '';
+  }
+  if (strings.length === 1) {
+    return strings[0];
+  }
+  const sorted = strings.slice().sort();
+  const min = sorted[0];
+  const max = sorted[sorted.length - 1];
+  for (let i = 0; i < min.length; i++) {
+    if (min[i] !== max[i]) {
+      return min.slice(0, i);
+    }
+  }
+  return min;
+}
+
 module.exports = {
+  commonPrefix,
   emitKeys,
   getStringWidth,
   isFullWidthCodePoint,

diff --git a/lib/readline.js b/lib/readline.js
@@ -48,6 +48,7 @@ const { validateString } = require('internal/validators');
 const { inspect } = require('internal/util/inspect');
 const EventEmitter = require('events');
 const {
+  commonPrefix,
   CSI,
   emitKeys,
   getStringWidth,
@@ -59,8 +60,8 @@ const {
 const { clearTimeout, setTimeout } = require('timers');
 const {
   kEscape,
-  kClearToBeginning,
-  kClearToEnd,
+  kClearToLineBeginning,
+  kClearToLineEnd,
   kClearLine,
   kClearScreenDown
 } = CSI;
@@ -567,23 +568,6 @@ function handleGroup(self, group, width, maxColumns) {
   self._writeToOutput('\r\n');
 }
 
-function commonPrefix(strings) {
-  if (!strings || strings.length === 0) {
-    return '';
-  }
-  if (strings.length === 1) return strings[0];
-  const sorted = strings.slice().sort();
-  const min = sorted[0];
-  const max = sorted[sorted.length - 1];
-  for (let i = 0, len = min.length; i < len; i++) {
-    if (min[i] !== max[i]) {
-      return min.slice(0, i);
-    }
-  }
-  return min;
-}
-
-
 Interface.prototype._wordLeft = function() {
   if (this.cursor > 0) {
     // Reverse the string and match a word near beginning
@@ -1268,7 +1252,11 @@ function clearLine(stream, dir, callback) {
     return true;
   }
 
-  const type = dir < 0 ? kClearToBeginning : dir > 0 ? kClearToEnd : kClearLine;
+  const type = dir < 0 ?
+    kClearToLineBeginning :
+    dir > 0 ?
+      kClearToLineEnd :
+      kClearLine;
   return stream.write(type, callback);
 }
 

diff --git a/lib/repl.js b/lib/repl.js
@@ -77,6 +77,9 @@ const vm = require('vm');
 const path = require('path');
 const fs = require('fs');
 const { Interface } = require('readline');
+const {
+  commonPrefix
+} = require('internal/readline/utils');
 const { Console } = require('console');
 const CJSModule = require('internal/modules/cjs/loader').Module;
 const domain = require('domain');
@@ -1387,25 +1390,6 @@ function complete(line, callback) {
   }
 }
 
-function longestCommonPrefix(arr = []) {
-  const cnt = arr.length;
-  if (cnt === 0) return '';
-  if (cnt === 1) return arr[0];
-
-  const first = arr[0];
-  // complexity: O(m * n)
-  for (let m = 0; m < first.length; m++) {
-    const c = first[m];
-    for (let n = 1; n < cnt; n++) {
-      const entry = arr[n];
-      if (m >= entry.length || c !== entry[m]) {
-        return first.substring(0, m);
-      }
-    }
-  }
-  return first;
-}
-
 REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
   if (err) return callback(err);
 
@@ -1418,7 +1402,7 @@ REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
   const trimCompleteOnPrefix = (v) => v.substring(prefixLength);
   const data = completions.filter(isNotEmpty).map(trimCompleteOnPrefix);
 
-  callback(null, [[`${completeOn}${longestCommonPrefix(data)}`], completeOn]);
+  callback(null, [[`${completeOn}${commonPrefix(data)}`], completeOn]);
 };
 
 REPLServer.prototype.defineCommand = function(keyword, cmd) {

diff --git a/test/parallel/test-readline-csi.js b/test/parallel/test-readline-csi.js
@@ -9,8 +9,8 @@ const { CSI } = require('internal/readline/utils');
 
 {
   assert(CSI);
-  assert.strictEqual(CSI.kClearToBeginning, '\x1b[1K');
-  assert.strictEqual(CSI.kClearToEnd, '\x1b[0K');
+  assert.strictEqual(CSI.kClearToLineBeginning, '\x1b[1K');
+  assert.strictEqual(CSI.kClearToLineEnd, '\x1b[0K');
   assert.strictEqual(CSI.kClearLine, '\x1b[2K');
   assert.strictEqual(CSI.kClearScreenDown, '\x1b[0J');
   assert.strictEqual(CSI`1${2}3`, '\x1b[123');
@@ -45,19 +45,19 @@ assert.strictEqual(readline.clearScreenDown(undefined, common.mustCall()),
 
 writable.data = '';
 assert.strictEqual(readline.clearLine(writable, -1), true);
-assert.deepStrictEqual(writable.data, CSI.kClearToBeginning);
+assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);
 
 writable.data = '';
 assert.strictEqual(readline.clearLine(writable, 1), true);
-assert.deepStrictEqual(writable.data, CSI.kClearToEnd);
+assert.deepStrictEqual(writable.data, CSI.kClearToLineEnd);
 
 writable.data = '';
 assert.strictEqual(readline.clearLine(writable, 0), true);
 assert.deepStrictEqual(writable.data, CSI.kClearLine);
 
 writable.data = '';
 assert.strictEqual(readline.clearLine(writable, -1, common.mustCall()), true);
-assert.deepStrictEqual(writable.data, CSI.kClearToBeginning);
+assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);
 
 // Verify that clearLine() throws on invalid callback.
 assert.throws(() => {