Skip to content

Commit

Permalink
repl,readline: refactor common code
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
BridgeAR authored and targos committed Apr 28, 2020
1 parent 0bafe08 commit 6eda28c
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 47 deletions.
24 changes: 22 additions & 2 deletions lib/internal/readline/utils.js
Expand Up @@ -32,8 +32,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`;

Expand Down Expand Up @@ -446,7 +446,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,
Expand Down
28 changes: 8 additions & 20 deletions lib/readline.js
Expand Up @@ -49,6 +49,7 @@ const { validateString } = require('internal/validators');
const { inspect } = require('internal/util/inspect');
const EventEmitter = require('events');
const {
commonPrefix,
CSI,
emitKeys,
getStringWidth,
Expand All @@ -60,8 +61,8 @@ const {
const { clearTimeout, setTimeout } = require('timers');
const {
kEscape,
kClearToBeginning,
kClearToEnd,
kClearToLineBeginning,
kClearToLineEnd,
kClearLine,
kClearScreenDown
} = CSI;
Expand Down Expand Up @@ -568,23 +569,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
Expand Down Expand Up @@ -1270,7 +1254,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);
}

Expand Down
24 changes: 4 additions & 20 deletions lib/repl.js
Expand Up @@ -84,6 +84,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 cjsLoader = require('internal/modules/cjs/loader');
const { Module: CJSModule } = cjsLoader;
Expand Down Expand Up @@ -1401,25 +1404,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);

Expand All @@ -1432,7 +1416,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) {
Expand Down
10 changes: 5 additions & 5 deletions test/parallel/test-readline-csi.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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(() => {
Expand Down

0 comments on commit 6eda28c

Please sign in to comment.