Skip to content

Commit f6f298e

Browse files
BridgeARMylesBorins
authored andcommitted
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>
1 parent 424c37b commit f6f298e

File tree

4 files changed

+39
-47
lines changed

4 files changed

+39
-47
lines changed

lib/internal/readline/utils.js

+22-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ function CSI(strings, ...args) {
3131
}
3232

3333
CSI.kEscape = kEscape;
34-
CSI.kClearToBeginning = CSI`1K`;
35-
CSI.kClearToEnd = CSI`0K`;
34+
CSI.kClearToLineBeginning = CSI`1K`;
35+
CSI.kClearToLineEnd = CSI`0K`;
3636
CSI.kClearLine = CSI`2K`;
3737
CSI.kClearScreenDown = CSI`0J`;
3838

@@ -445,7 +445,27 @@ function* emitKeys(stream) {
445445
}
446446
}
447447

448+
// This runs in O(n log n).
449+
function commonPrefix(strings) {
450+
if (!strings || strings.length === 0) {
451+
return '';
452+
}
453+
if (strings.length === 1) {
454+
return strings[0];
455+
}
456+
const sorted = strings.slice().sort();
457+
const min = sorted[0];
458+
const max = sorted[sorted.length - 1];
459+
for (let i = 0; i < min.length; i++) {
460+
if (min[i] !== max[i]) {
461+
return min.slice(0, i);
462+
}
463+
}
464+
return min;
465+
}
466+
448467
module.exports = {
468+
commonPrefix,
449469
emitKeys,
450470
getStringWidth,
451471
isFullWidthCodePoint,

lib/readline.js

+8-20
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const { validateString } = require('internal/validators');
4848
const { inspect } = require('internal/util/inspect');
4949
const EventEmitter = require('events');
5050
const {
51+
commonPrefix,
5152
CSI,
5253
emitKeys,
5354
getStringWidth,
@@ -59,8 +60,8 @@ const {
5960
const { clearTimeout, setTimeout } = require('timers');
6061
const {
6162
kEscape,
62-
kClearToBeginning,
63-
kClearToEnd,
63+
kClearToLineBeginning,
64+
kClearToLineEnd,
6465
kClearLine,
6566
kClearScreenDown
6667
} = CSI;
@@ -567,23 +568,6 @@ function handleGroup(self, group, width, maxColumns) {
567568
self._writeToOutput('\r\n');
568569
}
569570

570-
function commonPrefix(strings) {
571-
if (!strings || strings.length === 0) {
572-
return '';
573-
}
574-
if (strings.length === 1) return strings[0];
575-
const sorted = strings.slice().sort();
576-
const min = sorted[0];
577-
const max = sorted[sorted.length - 1];
578-
for (let i = 0, len = min.length; i < len; i++) {
579-
if (min[i] !== max[i]) {
580-
return min.slice(0, i);
581-
}
582-
}
583-
return min;
584-
}
585-
586-
587571
Interface.prototype._wordLeft = function() {
588572
if (this.cursor > 0) {
589573
// Reverse the string and match a word near beginning
@@ -1268,7 +1252,11 @@ function clearLine(stream, dir, callback) {
12681252
return true;
12691253
}
12701254

1271-
const type = dir < 0 ? kClearToBeginning : dir > 0 ? kClearToEnd : kClearLine;
1255+
const type = dir < 0 ?
1256+
kClearToLineBeginning :
1257+
dir > 0 ?
1258+
kClearToLineEnd :
1259+
kClearLine;
12721260
return stream.write(type, callback);
12731261
}
12741262

lib/repl.js

+4-20
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ const vm = require('vm');
7777
const path = require('path');
7878
const fs = require('fs');
7979
const { Interface } = require('readline');
80+
const {
81+
commonPrefix
82+
} = require('internal/readline/utils');
8083
const { Console } = require('console');
8184
const CJSModule = require('internal/modules/cjs/loader').Module;
8285
const domain = require('domain');
@@ -1387,25 +1390,6 @@ function complete(line, callback) {
13871390
}
13881391
}
13891392

1390-
function longestCommonPrefix(arr = []) {
1391-
const cnt = arr.length;
1392-
if (cnt === 0) return '';
1393-
if (cnt === 1) return arr[0];
1394-
1395-
const first = arr[0];
1396-
// complexity: O(m * n)
1397-
for (let m = 0; m < first.length; m++) {
1398-
const c = first[m];
1399-
for (let n = 1; n < cnt; n++) {
1400-
const entry = arr[n];
1401-
if (m >= entry.length || c !== entry[m]) {
1402-
return first.substring(0, m);
1403-
}
1404-
}
1405-
}
1406-
return first;
1407-
}
1408-
14091393
REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
14101394
if (err) return callback(err);
14111395

@@ -1418,7 +1402,7 @@ REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
14181402
const trimCompleteOnPrefix = (v) => v.substring(prefixLength);
14191403
const data = completions.filter(isNotEmpty).map(trimCompleteOnPrefix);
14201404

1421-
callback(null, [[`${completeOn}${longestCommonPrefix(data)}`], completeOn]);
1405+
callback(null, [[`${completeOn}${commonPrefix(data)}`], completeOn]);
14221406
};
14231407

14241408
REPLServer.prototype.defineCommand = function(keyword, cmd) {

test/parallel/test-readline-csi.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ const { CSI } = require('internal/readline/utils');
99

1010
{
1111
assert(CSI);
12-
assert.strictEqual(CSI.kClearToBeginning, '\x1b[1K');
13-
assert.strictEqual(CSI.kClearToEnd, '\x1b[0K');
12+
assert.strictEqual(CSI.kClearToLineBeginning, '\x1b[1K');
13+
assert.strictEqual(CSI.kClearToLineEnd, '\x1b[0K');
1414
assert.strictEqual(CSI.kClearLine, '\x1b[2K');
1515
assert.strictEqual(CSI.kClearScreenDown, '\x1b[0J');
1616
assert.strictEqual(CSI`1${2}3`, '\x1b[123');
@@ -45,19 +45,19 @@ assert.strictEqual(readline.clearScreenDown(undefined, common.mustCall()),
4545

4646
writable.data = '';
4747
assert.strictEqual(readline.clearLine(writable, -1), true);
48-
assert.deepStrictEqual(writable.data, CSI.kClearToBeginning);
48+
assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);
4949

5050
writable.data = '';
5151
assert.strictEqual(readline.clearLine(writable, 1), true);
52-
assert.deepStrictEqual(writable.data, CSI.kClearToEnd);
52+
assert.deepStrictEqual(writable.data, CSI.kClearToLineEnd);
5353

5454
writable.data = '';
5555
assert.strictEqual(readline.clearLine(writable, 0), true);
5656
assert.deepStrictEqual(writable.data, CSI.kClearLine);
5757

5858
writable.data = '';
5959
assert.strictEqual(readline.clearLine(writable, -1, common.mustCall()), true);
60-
assert.deepStrictEqual(writable.data, CSI.kClearToBeginning);
60+
assert.deepStrictEqual(writable.data, CSI.kClearToLineBeginning);
6161

6262
// Verify that clearLine() throws on invalid callback.
6363
assert.throws(() => {

0 commit comments

Comments
 (0)