From f6f298e3cf3ba21c51a5a5c686f3c0bb9e58e0a3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:21:40 +0100 Subject: [PATCH] repl,readline: refactor common code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/30907 Reviewed-By: Michaël Zasso Reviewed-By: Rich Trott --- lib/internal/readline/utils.js | 24 ++++++++++++++++++++++-- lib/readline.js | 28 ++++++++-------------------- lib/repl.js | 24 ++++-------------------- test/parallel/test-readline-csi.js | 10 +++++----- 4 files changed, 39 insertions(+), 47 deletions(-) diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js index 084976f440dfeb..ee3a477744d10e 100644 --- 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 index ea53cd1bbf80c8..9a306c654b50a5 100644 --- 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 index 1e7ab29bac03ab..221b5f1f612d92 100644 --- 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 index 27bfd2bce5173b..53b07d7bd939fd 100644 --- 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,11 +45,11 @@ 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); @@ -57,7 +57,7 @@ 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(() => {