From 19dac9855034e92f0f6feaadbdcb38205eb8628a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 10 Dec 2019 17:27:22 +0100 Subject: [PATCH 01/12] repl: remove dead code The .scope command was used only in the old debugger. Since that's not part of core anymore it's does not have any use. I tried to replicate the expected behavior but it even results in just exiting the repl immediately when using the completion similar to the removed test case. --- lib/repl.js | 48 +++++++++------------------ test/parallel/test-repl-eval-scope.js | 24 -------------- 2 files changed, 15 insertions(+), 57 deletions(-) delete mode 100644 test/parallel/test-repl-eval-scope.js diff --git a/lib/repl.js b/lib/repl.js index 4d40da4daabd95..5d932dda3f7052 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -43,7 +43,6 @@ 'use strict'; const { - ArrayIsArray, MathMax, NumberIsNaN, ObjectAssign, @@ -1277,40 +1276,23 @@ function complete(line, callback) { // Resolve expr and get its completions. const memberGroups = []; if (!expr) { - // If context is instance of vm.ScriptContext // Get global vars synchronously - if (this.useGlobal || vm.isContext(this.context)) { - completionGroups.push(getGlobalLexicalScopeNames(this[kContextId])); - let contextProto = this.context; - while (contextProto = ObjectGetPrototypeOf(contextProto)) { - completionGroups.push( - filteredOwnPropertyNames.call(this, contextProto)); - } - const contextOwnNames = - filteredOwnPropertyNames.call(this, this.context); - if (!this.useGlobal) { - // When the context is not `global`, builtins are not own - // properties of it. - contextOwnNames.push(...globalBuiltins); - } - completionGroups.push(contextOwnNames); - if (filter !== '') addCommonWords(completionGroups); - completionGroupsLoaded(); - } else { - this.eval('.scope', this.context, 'repl', function ev(err, globals) { - if (err || !ArrayIsArray(globals)) { - if (filter !== '') addCommonWords(completionGroups); - } else if (ArrayIsArray(globals[0])) { - // Add grouped globals - for (let n = 0; n < globals.length; n++) - completionGroups.push(globals[n]); - } else { - completionGroups.push(globals); - if (filter !== '') addCommonWords(completionGroups); - } - completionGroupsLoaded(); - }); + completionGroups.push(getGlobalLexicalScopeNames(this[kContextId])); + let contextProto = this.context; + while (contextProto = ObjectGetPrototypeOf(contextProto)) { + completionGroups.push( + filteredOwnPropertyNames.call(this, contextProto)); + } + const contextOwnNames = + filteredOwnPropertyNames.call(this, this.context); + if (!this.useGlobal) { + // When the context is not `global`, builtins are not own + // properties of it. + contextOwnNames.push(...globalBuiltins); } + completionGroups.push(contextOwnNames); + if (filter !== '') addCommonWords(completionGroups); + completionGroupsLoaded(); } else { const evalExpr = `try { ${expr} } catch {}`; this.eval(evalExpr, this.context, 'repl', (e, obj) => { diff --git a/test/parallel/test-repl-eval-scope.js b/test/parallel/test-repl-eval-scope.js deleted file mode 100644 index 702b6056f101a5..00000000000000 --- a/test/parallel/test-repl-eval-scope.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; -const common = require('../common'); -const ArrayStream = require('../common/arraystream'); -const assert = require('assert'); -const repl = require('repl'); - -{ - const stream = new ArrayStream(); - const options = { - eval: common.mustCall((cmd, context) => { - assert.strictEqual(cmd, '.scope\n'); - assert.deepStrictEqual(context, { animal: 'Sterrance' }); - }), - input: stream, - output: stream, - terminal: true - }; - - const r = repl.start(options); - r.context = { animal: 'Sterrance' }; - - stream.emit('data', '\t'); - stream.emit('.exit\n'); -} From c65f3f1384865ceff9dc08db5c86b4de47aaf02f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 10 Dec 2019 17:31:13 +0100 Subject: [PATCH 02/12] repl: simplify repl autocompletion This simplifies calling `filteredOwnPropertyNames()`. The context is not used in that function, so there's no need to call the function as such. --- lib/repl.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 5d932dda3f7052..eeee83fee3c70f 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -1280,11 +1280,9 @@ function complete(line, callback) { completionGroups.push(getGlobalLexicalScopeNames(this[kContextId])); let contextProto = this.context; while (contextProto = ObjectGetPrototypeOf(contextProto)) { - completionGroups.push( - filteredOwnPropertyNames.call(this, contextProto)); + completionGroups.push(filteredOwnPropertyNames(contextProto)); } - const contextOwnNames = - filteredOwnPropertyNames.call(this, this.context); + const contextOwnNames = filteredOwnPropertyNames(this.context); if (!this.useGlobal) { // When the context is not `global`, builtins are not own // properties of it. @@ -1299,7 +1297,7 @@ function complete(line, callback) { if (obj != null) { if (typeof obj === 'object' || typeof obj === 'function') { try { - memberGroups.push(filteredOwnPropertyNames.call(this, obj)); + memberGroups.push(filteredOwnPropertyNames(obj)); } catch { // Probably a Proxy object without `getOwnPropertyNames` trap. // We simply ignore it here, as we don't want to break the @@ -1317,7 +1315,7 @@ function complete(line, callback) { p = obj.constructor ? obj.constructor.prototype : null; } while (p !== null) { - memberGroups.push(filteredOwnPropertyNames.call(this, p)); + memberGroups.push(filteredOwnPropertyNames(p)); p = ObjectGetPrototypeOf(p); // Circular refs possible? Let's guard against that. sentinel--; From ec47482391ecfedfd0d910f03658e002789d3515 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 10 Dec 2019 18:17:01 +0100 Subject: [PATCH 03/12] repl: simplify code This simplifies some repl code and removes a coe branch that is unreachable. --- lib/repl.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index eeee83fee3c70f..1e7ab29bac03ab 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -953,7 +953,7 @@ REPLServer.prototype.createContext = function() { } const module = new CJSModule(''); - module.paths = CJSModule._resolveLookupPaths('', parentModule) || []; + module.paths = CJSModule._resolveLookupPaths('', parentModule); ObjectDefineProperty(context, 'module', { configurable: true, @@ -1307,21 +1307,17 @@ function complete(line, callback) { } // Works for non-objects try { - let sentinel = 5; let p; if (typeof obj === 'object' || typeof obj === 'function') { p = ObjectGetPrototypeOf(obj); } else { p = obj.constructor ? obj.constructor.prototype : null; } - while (p !== null) { + // Circular refs possible? Let's guard against that. + let sentinel = 5; + while (p !== null && sentinel-- !== 0) { memberGroups.push(filteredOwnPropertyNames(p)); p = ObjectGetPrototypeOf(p); - // Circular refs possible? Let's guard against that. - sentinel--; - if (sentinel <= 0) { - break; - } } } catch {} } From 43ae68937b5f39f15b9b1bebda68c34e41a53ee0 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 11:12:26 +0100 Subject: [PATCH 04/12] readline: update ansi-regex This updates the used regular expression to the latest version. It includes a number of additional escape codes. --- lib/internal/readline/utils.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/internal/readline/utils.js b/lib/internal/readline/utils.js index 510acf221831a0..084976f440dfeb 100644 --- a/lib/internal/readline/utils.js +++ b/lib/internal/readline/utils.js @@ -7,12 +7,13 @@ const { // Regex used for ansi escape code splitting // Adopted from https://github.com/chalk/ansi-regex/blob/master/index.js -// License: MIT, authors: @sindresorhus, Qix-, and arjunmehta +// License: MIT, authors: @sindresorhus, Qix-, arjunmehta and LitoMore // Matches all ansi escape code sequences in a string -/* eslint-disable no-control-regex */ -const ansi = - /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g; -/* eslint-enable no-control-regex */ +const ansiPattern = '[\\u001B\\u009B][[\\]()#;?]*' + + '(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)' + + '|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))'; +const ansi = new RegExp(ansiPattern, 'g'); + const kUTF16SurrogateThreshold = 0x10000; // 2 ** 16 const kEscape = '\x1b'; From 1426e6c2714a52f3b7af581bc1ef227ecc05152a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:21:40 +0100 Subject: [PATCH 05/12] 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. --- 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(() => { From 42d3590aedd111ae14bdabcd256241f7709fca42 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:24:39 +0100 Subject: [PATCH 06/12] repl,readline: refactor for simplicity This just refactors code without changing the behavior. Especially the REPL code is difficult to read and deeply indented. This reduces the indentation to improve that. --- lib/readline.js | 64 ++++++----- lib/repl.js | 296 ++++++++++++++++++++++++------------------------ 2 files changed, 179 insertions(+), 181 deletions(-) diff --git a/lib/readline.js b/lib/readline.js index 9a306c654b50a5..0ba30cc04b1124 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -505,41 +505,43 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) { return; } - const completions = rv[0]; - const completeOn = rv[1]; // The text that was completed - if (completions && completions.length) { - // Apply/show completions. - if (lastKeypressWasTab) { - self._writeToOutput('\r\n'); - const width = completions.reduce(function completionReducer(a, b) { - return a.length > b.length ? a : b; - }).length + 2; // 2 space padding - let maxColumns = MathFloor(self.columns / width); - if (!maxColumns || maxColumns === Infinity) { - maxColumns = 1; - } - let group = []; - for (let i = 0; i < completions.length; i++) { - const c = completions[i]; - if (c === '') { - handleGroup(self, group, width, maxColumns); - group = []; - } else { - group.push(c); - } - } - handleGroup(self, group, width, maxColumns); - } + // Result and the text that was completed. + const [completions, completeOn] = rv; + + if (!completions || completions.length === 0) { + return; + } - // If there is a common prefix to all matches, then apply that portion. - const f = completions.filter((e) => e); - const prefix = commonPrefix(f); - if (prefix.length > completeOn.length) { - self._insertString(prefix.slice(completeOn.length)); + // Apply/show completions. + if (lastKeypressWasTab) { + self._writeToOutput('\r\n'); + const width = completions.reduce((a, b) => { + return a.length > b.length ? a : b; + }).length + 2; // 2 space padding + let maxColumns = MathFloor(self.columns / width); + if (!maxColumns || maxColumns === Infinity) { + maxColumns = 1; } + let group = []; + for (const c of completions) { + if (c === '') { + handleGroup(self, group, width, maxColumns); + group = []; + } else { + group.push(c); + } + } + handleGroup(self, group, width, maxColumns); + } - self._refreshLine(); + // If there is a common prefix to all matches, then apply that portion. + const f = completions.filter((e) => e); + const prefix = commonPrefix(f); + if (prefix.length > completeOn.length) { + self._insertString(prefix.slice(completeOn.length)); } + + self._refreshLine(); }); }; diff --git a/lib/repl.js b/lib/repl.js index 221b5f1f612d92..4f32c9c28ced3d 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -52,7 +52,6 @@ const { ObjectGetOwnPropertyNames, ObjectGetPrototypeOf, ObjectKeys, - ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, Symbol, } = primordials; @@ -1164,7 +1163,7 @@ function complete(line, callback) { completeOn = match[1]; const subdir = match[2] || ''; filter = match[1]; - let dir, files, name, base, ext, abs, subfiles, isDirectory; + let dir, files, subfiles, isDirectory; group = []; let paths = []; @@ -1186,14 +1185,14 @@ function complete(line, callback) { continue; } for (let f = 0; f < files.length; f++) { - name = files[f]; - ext = path.extname(name); - base = name.slice(0, -ext.length); + const name = files[f]; + const ext = path.extname(name); + const base = name.slice(0, -ext.length); if (versionedFileNamesRe.test(base) || name === '.npm') { // Exclude versioned names that 'npm' installs. continue; } - abs = path.resolve(dir, name); + const abs = path.resolve(dir, name); try { isDirectory = fs.statSync(abs).isDirectory(); } catch { @@ -1261,86 +1260,87 @@ function complete(line, callback) { // foo.<|> # completions for 'foo' with filter '' } else if (line.length === 0 || /\w|\.|\$/.test(line[line.length - 1])) { match = simpleExpressionRE.exec(line); - if (line.length === 0 || match) { - let expr; - completeOn = (match ? match[0] : ''); - if (line.length === 0) { - filter = ''; - expr = ''; - } else if (line[line.length - 1] === '.') { - filter = ''; - expr = match[0].slice(0, match[0].length - 1); - } else { - const bits = match[0].split('.'); - filter = bits.pop(); - expr = bits.join('.'); + if (line.length !== 0 && !match) { + completionGroupsLoaded(); + return; + } + let expr; + completeOn = (match ? match[0] : ''); + if (line.length === 0) { + filter = ''; + expr = ''; + } else if (line[line.length - 1] === '.') { + filter = ''; + expr = match[0].slice(0, match[0].length - 1); + } else { + const bits = match[0].split('.'); + filter = bits.pop(); + expr = bits.join('.'); + } + + // Resolve expr and get its completions. + const memberGroups = []; + if (!expr) { + // Get global vars synchronously + completionGroups.push(getGlobalLexicalScopeNames(this[kContextId])); + let contextProto = this.context; + while (contextProto = ObjectGetPrototypeOf(contextProto)) { + completionGroups.push(filteredOwnPropertyNames(contextProto)); } + const contextOwnNames = filteredOwnPropertyNames(this.context); + if (!this.useGlobal) { + // When the context is not `global`, builtins are not own + // properties of it. + contextOwnNames.push(...globalBuiltins); + } + completionGroups.push(contextOwnNames); + if (filter !== '') addCommonWords(completionGroups); + completionGroupsLoaded(); + return; + } - // Resolve expr and get its completions. - const memberGroups = []; - if (!expr) { - // Get global vars synchronously - completionGroups.push(getGlobalLexicalScopeNames(this[kContextId])); - let contextProto = this.context; - while (contextProto = ObjectGetPrototypeOf(contextProto)) { - completionGroups.push(filteredOwnPropertyNames(contextProto)); - } - const contextOwnNames = filteredOwnPropertyNames(this.context); - if (!this.useGlobal) { - // When the context is not `global`, builtins are not own - // properties of it. - contextOwnNames.push(...globalBuiltins); + const evalExpr = `try { ${expr} } catch {}`; + this.eval(evalExpr, this.context, 'repl', (e, obj) => { + if (obj != null) { + if (typeof obj === 'object' || typeof obj === 'function') { + try { + memberGroups.push(filteredOwnPropertyNames(obj)); + } catch { + // Probably a Proxy object without `getOwnPropertyNames` trap. + // We simply ignore it here, as we don't want to break the + // autocompletion. Fixes the bug + // https://github.com/nodejs/node/issues/2119 + } } - completionGroups.push(contextOwnNames); - if (filter !== '') addCommonWords(completionGroups); - completionGroupsLoaded(); - } else { - const evalExpr = `try { ${expr} } catch {}`; - this.eval(evalExpr, this.context, 'repl', (e, obj) => { - if (obj != null) { - if (typeof obj === 'object' || typeof obj === 'function') { - try { - memberGroups.push(filteredOwnPropertyNames(obj)); - } catch { - // Probably a Proxy object without `getOwnPropertyNames` trap. - // We simply ignore it here, as we don't want to break the - // autocompletion. Fixes the bug - // https://github.com/nodejs/node/issues/2119 - } - } - // Works for non-objects - try { - let p; - if (typeof obj === 'object' || typeof obj === 'function') { - p = ObjectGetPrototypeOf(obj); - } else { - p = obj.constructor ? obj.constructor.prototype : null; - } - // Circular refs possible? Let's guard against that. - let sentinel = 5; - while (p !== null && sentinel-- !== 0) { - memberGroups.push(filteredOwnPropertyNames(p)); - p = ObjectGetPrototypeOf(p); - } - } catch {} + // Works for non-objects + try { + let p; + if (typeof obj === 'object' || typeof obj === 'function') { + p = ObjectGetPrototypeOf(obj); + } else { + p = obj.constructor ? obj.constructor.prototype : null; } - - if (memberGroups.length) { - for (let i = 0; i < memberGroups.length; i++) { - completionGroups.push( - memberGroups[i].map((member) => `${expr}.${member}`)); - } - if (filter) { - filter = `${expr}.${filter}`; - } + // Circular refs possible? Let's guard against that. + let sentinel = 5; + while (p !== null && sentinel-- !== 0) { + memberGroups.push(filteredOwnPropertyNames(p)); + p = ObjectGetPrototypeOf(p); } + } catch {} + } - completionGroupsLoaded(); - }); + if (memberGroups.length) { + for (let i = 0; i < memberGroups.length; i++) { + completionGroups.push( + memberGroups[i].map((member) => `${expr}.${member}`)); + } + if (filter) { + filter = `${expr}.${filter}`; + } } - } else { + completionGroupsLoaded(); - } + }); } else { completionGroupsLoaded(); } @@ -1361,32 +1361,32 @@ function complete(line, callback) { completionGroups = newCompletionGroups; } - let completions; - - if (completionGroups.length) { - const uniq = {}; // Unique completions across all groups - completions = []; - // Completion group 0 is the "closest" - // (least far up the inheritance chain) - // so we put its completions last: to be closest in the REPL. - for (let i = 0; i < completionGroups.length; i++) { - group = completionGroups[i]; - group.sort(); - for (let j = group.length - 1; j >= 0; j--) { - const c = group[j]; - if (!ObjectPrototypeHasOwnProperty(uniq, c)) { - completions.unshift(c); - uniq[c] = true; - } + const completions = []; + // Unique completions across all groups. + const uniqueSet = new Set(['']); + // Completion group 0 is the "closest" (least far up the inheritance + // chain) so we put its completions last: to be closest in the REPL. + for (const group of completionGroups) { + group.sort((a, b) => (b > a ? 1 : -1)); + const setSize = uniqueSet.size; + for (const entry of group) { + if (!uniqueSet.has(entry)) { + completions.unshift(entry); + uniqueSet.add(entry); } - completions.unshift(''); // Separator btwn groups } - while (completions.length && completions[0] === '') { - completions.shift(); + // Add a separator between groups. + if (uniqueSet.size !== setSize) { + completions.unshift(''); } } - callback(null, [completions || [], completeOn]); + // Remove obsolete group entry, if present. + if (completions[0] === '') { + completions.shift(); + } + + callback(null, [completions, completeOn]); } } @@ -1419,6 +1419,9 @@ REPLServer.prototype.memory = deprecate( 'REPLServer.memory() is deprecated', 'DEP0082'); +// TODO(BridgeAR): This should be replaced with acorn to build an AST. The +// language became more complex and using a simple approach like this is not +// sufficient anymore. function _memory(cmd) { const self = this; self.lines = self.lines || []; @@ -1426,7 +1429,6 @@ function _memory(cmd) { // Save the line so I can do magic later if (cmd) { - // TODO should I tab the level? const len = self.lines.level.length ? self.lines.level.length - 1 : 0; self.lines.push(' '.repeat(len) + cmd); } else { @@ -1434,61 +1436,55 @@ function _memory(cmd) { self.lines.push(''); } + if (!cmd) { + self.lines.level = []; + return; + } + // I need to know "depth." // Because I can not tell the difference between a } that // closes an object literal and a } that closes a function - if (cmd) { - // Going down is { and ( e.g. function() { - // going up is } and ) - let dw = cmd.match(/[{(]/g); - let up = cmd.match(/[})]/g); - up = up ? up.length : 0; - dw = dw ? dw.length : 0; - let depth = dw - up; - - if (depth) { - (function workIt() { - if (depth > 0) { - // Going... down. - // Push the line#, depth count, and if the line is a function. - // Since JS only has functional scope I only need to remove - // "function() {" lines, clearly this will not work for - // "function() - // {" but nothing should break, only tab completion for local - // scope will not work for this function. - self.lines.level.push({ - line: self.lines.length - 1, - depth: depth, - isFunction: /\bfunction\b/.test(cmd) - }); - } else if (depth < 0) { - // Going... up. - const curr = self.lines.level.pop(); - if (curr) { - const tmp = curr.depth + depth; - if (tmp < 0) { - // More to go, recurse - depth += curr.depth; - workIt(); - } else if (tmp > 0) { - // Remove and push back - curr.depth += depth; - self.lines.level.push(curr); - } + + // Going down is { and ( e.g. function() { + // going up is } and ) + let dw = cmd.match(/[{(]/g); + let up = cmd.match(/[})]/g); + up = up ? up.length : 0; + dw = dw ? dw.length : 0; + let depth = dw - up; + + if (depth) { + (function workIt() { + if (depth > 0) { + // Going... down. + // Push the line#, depth count, and if the line is a function. + // Since JS only has functional scope I only need to remove + // "function() {" lines, clearly this will not work for + // "function() + // {" but nothing should break, only tab completion for local + // scope will not work for this function. + self.lines.level.push({ + line: self.lines.length - 1, + depth: depth, + isFunction: /\bfunction\b/.test(cmd) + }); + } else if (depth < 0) { + // Going... up. + const curr = self.lines.level.pop(); + if (curr) { + const tmp = curr.depth + depth; + if (tmp < 0) { + // More to go, recurse + depth += curr.depth; + workIt(); + } else if (tmp > 0) { + // Remove and push back + curr.depth += depth; + self.lines.level.push(curr); } } - }()); - } - - // It is possible to determine a syntax error at this point. - // if the REPL still has a bufferedCommand and - // self.lines.level.length === 0 - // TODO? keep a log of level so that any syntax breaking lines can - // be cleared on .break and in the case of a syntax error? - // TODO? if a log was kept, then I could clear the bufferedCommand and - // eval these lines and throw the syntax error - } else { - self.lines.level = []; + } + }()); } } From ca14259ff7c2e57d485efba0892727d202446aef Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:26:47 +0100 Subject: [PATCH 07/12] repl: improve completion This improves the completion output by removing the nested special handling. It never fully worked as expected and required a lot of hacks to even keep it working halfway reliable. Our tests did not cover syntax errors though and those can not be handled by this implementation. Those break the layout and confuse the REPL. Besides that the completion now also works in case the current line has leading whitespace. Also improve the error output in case the completion fails. --- lib/readline.js | 2 +- lib/repl.js | 70 +++---------------- .../repl-tab-completion-nested-repls.js | 2 +- test/parallel/test-repl-save-load.js | 5 +- .../test-repl-tab-complete-nested-repls.js | 2 + test/parallel/test-repl-tab-complete.js | 29 +++----- 6 files changed, 27 insertions(+), 83 deletions(-) diff --git a/lib/readline.js b/lib/readline.js index 0ba30cc04b1124..d471d512f485d5 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -501,7 +501,7 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) { self.resume(); if (err) { - self._writeToOutput(`tab completion error ${inspect(err)}`); + self._writeToOutput(`Tab completion error: ${inspect(err)}`); return; } diff --git a/lib/repl.js b/lib/repl.js index 4f32c9c28ced3d..cd8240dd743919 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -71,7 +71,6 @@ const { deprecate } = require('internal/util'); const { inspect } = require('internal/util/inspect'); -const Stream = require('stream'); const vm = require('vm'); const path = require('path'); const fs = require('fs'); @@ -115,7 +114,6 @@ const { } = internalBinding('contextify'); const history = require('internal/repl/history'); -const { setImmediate } = require('timers'); // Lazy-loaded. let processTopLevelAwait; @@ -124,7 +122,6 @@ const globalBuiltins = new Set(vm.runInNewContext('Object.getOwnPropertyNames(globalThis)')); const parentModule = module; -const replMap = new WeakMap(); const domainSet = new WeakSet(); const kBufferedCommandSymbol = Symbol('bufferedCommand'); @@ -550,14 +547,13 @@ function REPLServer(prompt, self.lastError = e; } - const top = replMap.get(self); if (options[kStandaloneREPL] && process.listenerCount('uncaughtException') !== 0) { process.nextTick(() => { process.emit('uncaughtException', e); - top.clearBufferedCommand(); - top.lines.level = []; - top.displayPrompt(); + self.clearBufferedCommand(); + self.lines.level = []; + self.displayPrompt(); }); } else { if (errStack === '') { @@ -583,10 +579,10 @@ function REPLServer(prompt, } // Normalize line endings. errStack += errStack.endsWith('\n') ? '' : '\n'; - top.outputStream.write(errStack); - top.clearBufferedCommand(); - top.lines.level = []; - top.displayPrompt(); + self.outputStream.write(errStack); + self.clearBufferedCommand(); + self.lines.level = []; + self.displayPrompt(); } }); @@ -897,7 +893,6 @@ exports.start = function(prompt, ignoreUndefined, replMode); if (!exports.repl) exports.repl = repl; - replMap.set(repl, repl); return repl; }; @@ -1034,23 +1029,6 @@ REPLServer.prototype.turnOffEditorMode = deprecate( 'REPLServer.turnOffEditorMode() is deprecated', 'DEP0078'); -// A stream to push an array into a REPL -// used in REPLServer.complete -function ArrayStream() { - Stream.call(this); - - this.run = function(data) { - for (let n = 0; n < data.length; n++) - this.emit('data', `${data[n]}\n`); - }; -} -ObjectSetPrototypeOf(ArrayStream.prototype, Stream.prototype); -ObjectSetPrototypeOf(ArrayStream, Stream); -ArrayStream.prototype.readable = true; -ArrayStream.prototype.writable = true; -ArrayStream.prototype.resume = function() {}; -ArrayStream.prototype.write = function() {}; - const requireRE = /\brequire\s*\(['"](([\w@./-]+\/)?(?:[\w@./-]*))/; const fsAutoCompleteRE = /fs(?:\.promises)?\.\s*[a-z][a-zA-Z]+\(\s*["'](.*)/; const simpleExpressionRE = @@ -1110,38 +1088,13 @@ REPLServer.prototype.complete = function() { // Warning: This eval's code like "foo.bar.baz", so it will run property // getter code. function complete(line, callback) { - // There may be local variables to evaluate, try a nested REPL - if (this[kBufferedCommandSymbol] !== undefined && - this[kBufferedCommandSymbol].length) { - // Get a new array of inputted lines - const tmp = this.lines.slice(); - // Kill off all function declarations to push all local variables into - // global scope - for (let n = 0; n < this.lines.level.length; n++) { - const kill = this.lines.level[n]; - if (kill.isFunction) - tmp[kill.line] = ''; - } - const flat = new ArrayStream(); // Make a new "input" stream. - const magic = new REPLServer('', flat); // Make a nested REPL. - replMap.set(magic, replMap.get(this)); - flat.run(tmp); // `eval` the flattened code. - // All this is only profitable if the nested REPL does not have a - // bufferedCommand. - if (!magic[kBufferedCommandSymbol]) { - magic._domain.on('error', (err) => { - setImmediate(() => { - throw err; - }); - }); - return magic.complete(line, callback); - } - } - // List of completion lists, one for each inheritance "level" let completionGroups = []; let completeOn, group; + // Ignore right whitespace. It could change the outcome. + line = line.trimLeft(); + // REPL commands (e.g. ".break"). let filter; let match = line.match(/^\s*\.(\w*)$/); @@ -1465,8 +1418,7 @@ function _memory(cmd) { // scope will not work for this function. self.lines.level.push({ line: self.lines.length - 1, - depth: depth, - isFunction: /\bfunction\b/.test(cmd) + depth: depth }); } else if (depth < 0) { // Going... up. diff --git a/test/fixtures/repl-tab-completion-nested-repls.js b/test/fixtures/repl-tab-completion-nested-repls.js index ceff6e79453705..1d2b154f2b3341 100644 --- a/test/fixtures/repl-tab-completion-nested-repls.js +++ b/test/fixtures/repl-tab-completion-nested-repls.js @@ -31,7 +31,7 @@ const repl = require('repl'); const putIn = new ArrayStream(); const testMe = repl.start('', putIn); -// Some errors are passed to the domain, but do not callback +// Some errors are passed to the domain, but do not callback. testMe._domain.on('error', function(err) { throw err; }); diff --git a/test/parallel/test-repl-save-load.js b/test/parallel/test-repl-save-load.js index ef9ff8f6498877..f6ecc8d4ab67e9 100644 --- a/test/parallel/test-repl-save-load.js +++ b/test/parallel/test-repl-save-load.js @@ -44,8 +44,9 @@ testMe._domain.on('error', function(reason) { }); const testFile = [ - 'let top = function() {', - 'let inner = {one:1};' + 'let inner = (function() {', + ' return {one:1};', + '})()' ]; const saveFileName = join(tmpdir.path, 'test.save.js'); diff --git a/test/parallel/test-repl-tab-complete-nested-repls.js b/test/parallel/test-repl-tab-complete-nested-repls.js index 36547e8d9fb5be..3cac02f20562bc 100644 --- a/test/parallel/test-repl-tab-complete-nested-repls.js +++ b/test/parallel/test-repl-tab-complete-nested-repls.js @@ -19,3 +19,5 @@ const result = spawnSync(process.execPath, [testFile]); // test here is to make sure that the error information bubbles up to the // calling process. assert.ok(result.status, 'testFile swallowed its error'); +const err = result.stderr.toString(); +assert.ok(err.includes('fhqwhgads'), err); diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 1c66f9a3238230..6cf689c4b11074 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -54,12 +54,9 @@ const putIn = new ArrayStream(); const testMe = repl.start('', putIn); // Some errors are passed to the domain, but do not callback -testMe._domain.on('error', function(err) { - assert.ifError(err); -}); +testMe._domain.on('error', assert.ifError); // Tab Complete will not break in an object literal -putIn.run(['.clear']); putIn.run([ 'var inner = {', 'one:1' @@ -93,9 +90,7 @@ putIn.run([ 'var top = function() {', 'var inner = {one:1};' ]); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); +testMe.complete('inner.o', getNoResultsFunction()); // When you close the function scope tab complete will not return the // locally scoped variable @@ -111,9 +106,7 @@ putIn.run([ ' one:1', '};' ]); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); +testMe.complete('inner.o', getNoResultsFunction()); putIn.run(['.clear']); @@ -125,9 +118,7 @@ putIn.run([ ' one:1', '};' ]); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); +testMe.complete('inner.o', getNoResultsFunction()); putIn.run(['.clear']); @@ -140,9 +131,7 @@ putIn.run([ ' one:1', '};' ]); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); +testMe.complete('inner.o', getNoResultsFunction()); putIn.run(['.clear']); @@ -155,9 +144,7 @@ putIn.run([ ' one:1', '};' ]); -testMe.complete('inner.o', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, works); -})); +testMe.complete('inner.o', getNoResultsFunction()); putIn.run(['.clear']); @@ -204,7 +191,9 @@ const spaceTimeout = setTimeout(function() { }, 1000); testMe.complete(' ', common.mustCall(function(error, data) { - assert.deepStrictEqual(data, [[], undefined]); + assert.ifError(error); + assert.strictEqual(data[1], ''); + assert.ok(data[0].includes('globalThis')); clearTimeout(spaceTimeout); })); From 1cbef42ee712149071b5824c7246169c887ef006 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:33:53 +0100 Subject: [PATCH 08/12] repl: add completion preview This improves the already existing preview functionality by also checking for the input completion. In case there's only a single completion, it will automatically be visible to the user in grey. If colors are deactivated, it will be visible as comment. This also changes some keys by automatically accepting the preview by moving the cursor behind the current input end. --- doc/api/repl.md | 5 +- lib/internal/repl/utils.js | 165 ++++++++++++++++++--- lib/readline.js | 8 +- lib/repl.js | 8 +- test/parallel/test-repl-editor.js | 9 +- test/parallel/test-repl-multiline.js | 13 +- test/parallel/test-repl-preview.js | 8 +- test/parallel/test-repl-top-level-await.js | 122 +++++++++------ 8 files changed, 257 insertions(+), 81 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 21f5193c12c947..fc9332c910cb36 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -565,8 +565,9 @@ changes: * `breakEvalOnSigint` {boolean} Stop evaluating the current piece of code when `SIGINT` is received, such as when `Ctrl+C` is pressed. This cannot be used together with a custom `eval` function. **Default:** `false`. - * `preview` {boolean} Defines if the repl prints output previews or not. - **Default:** `true`. Always `false` in case `terminal` is falsy. + * `preview` {boolean} Defines if the repl prints autocomplete and output + previews or not. **Default:** `true`. Always `false` in case `terminal` is + falsy. * Returns: {repl.REPLServer} The `repl.start()` method creates and starts a [`repl.REPLServer`][] instance. diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index c4280c1d1fe9e2..3deee9a57105b2 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -28,6 +28,10 @@ const { moveCursor, } = require('readline'); +const { + commonPrefix +} = require('internal/readline/utils'); + const { inspect } = require('util'); const debug = require('internal/util/debuglog').debuglog('repl'); @@ -119,24 +123,98 @@ function isRecoverableError(e, code) { function setupPreview(repl, contextSymbol, bufferSymbol, active) { // Simple terminals can't handle previews. if (process.env.TERM === 'dumb' || !active) { - return { showInputPreview() {}, clearPreview() {} }; + return { showPreview() {}, clearPreview() {} }; } - let preview = null; - let lastPreview = ''; + let inputPreview = null; + let lastInputPreview = ''; + + let previewCompletionCounter = 0; + let completionPreview = null; const clearPreview = () => { - if (preview !== null) { + if (inputPreview !== null) { moveCursor(repl.output, 0, 1); clearLine(repl.output); moveCursor(repl.output, 0, -1); - lastPreview = preview; - preview = null; + lastInputPreview = inputPreview; + inputPreview = null; + } + if (completionPreview !== null) { + // Prevent cursor moves if not necessary! + const move = repl.line.length !== repl.cursor; + if (move) { + cursorTo(repl.output, repl._prompt.length + repl.line.length); + } + clearLine(repl.output, 1); + if (move) { + cursorTo(repl.output, repl._prompt.length + repl.cursor); + } + completionPreview = null; } }; + function showCompletionPreview(line, insertPreview) { + previewCompletionCounter++; + + const count = previewCompletionCounter; + + repl.completer(line, (error, data) => { + // Tab completion might be async and the result might already be outdated. + if (count !== previewCompletionCounter) { + return; + } + + if (error) { + debug('Error while generating completion preview', error); + return; + } + + // Result and the text that was completed. + const [rawCompletions, completeOn] = data; + + if (!rawCompletions || rawCompletions.length === 0) { + return; + } + + // If there is a common prefix to all matches, then apply that portion. + const completions = rawCompletions.filter((e) => e); + const prefix = commonPrefix(completions); + + // No common prefix found. + if (prefix.length <= completeOn.length) { + return; + } + + const suffix = prefix.slice(completeOn.length); + + // TODO(BridgeAR): Fix me. This should not be necessary. See similar + // comment in `showPreview()`. + if (repl.line.length + repl._prompt.length + suffix > repl.columns) { + return; + } + + if (insertPreview) { + repl._insertString(suffix); + return; + } + + completionPreview = suffix; + + const result = repl.useColors ? + `\u001b[90m${suffix}\u001b[39m` : + ` // ${suffix}`; + + if (repl.line.length !== repl.cursor) { + cursorTo(repl.output, repl._prompt.length + repl.line.length); + } + repl.output.write(result); + cursorTo(repl.output, repl._prompt.length + repl.cursor); + }); + } + // This returns a code preview for arbitrary input code. - function getPreviewInput(input, callback) { + function getInputPreview(input, callback) { // For similar reasons as `defaultEval`, wrap expressions starting with a // curly brace with parenthesis. if (input.startsWith('{') && !input.endsWith(';')) { @@ -184,23 +262,41 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { }, () => callback(new ERR_INSPECTOR_NOT_AVAILABLE())); } - const showInputPreview = () => { + const showPreview = () => { // Prevent duplicated previews after a refresh. - if (preview !== null) { + if (inputPreview !== null) { return; } const line = repl.line.trim(); - // Do not preview if the command is buffered or if the line is empty. - if (repl[bufferSymbol] || line === '') { + // Do not preview in case the line only contains whitespace. + if (line === '') { + return; + } + + // Add the autocompletion preview. + // TODO(BridgeAR): Trigger the input preview after the completion preview. + // That way it's possible to trigger the input prefix including the + // potential completion suffix. To do so, we also have to change the + // behavior of `enter` and `escape`: + // Enter should automatically add the suffix to the current line as long as + // escape was not pressed. We might even remove the preview in case any + // cursor movement is triggered. + if (typeof repl.completer === 'function') { + const insertPreview = false; + showCompletionPreview(repl.line, insertPreview); + } + + // Do not preview if the command is buffered. + if (repl[bufferSymbol]) { return; } - getPreviewInput(line, (error, inspected) => { + getInputPreview(line, (error, inspected) => { // Ignore the output if the value is identical to the current line and the // former preview is not identical to this preview. - if ((line === inspected && lastPreview !== inspected) || + if ((line === inspected && lastInputPreview !== inspected) || inspected === null) { return; } @@ -215,7 +311,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { return; } - preview = inspected; + inputPreview = inspected; // Limit the output to maximum 250 characters. Otherwise it becomes a) // difficult to read and b) non terminal REPLs would visualize the whole @@ -235,21 +331,50 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { repl.output.write(`\n${result}`); moveCursor(repl.output, 0, -1); - cursorTo(repl.output, repl.cursor + repl._prompt.length); + cursorTo(repl.output, repl._prompt.length + repl.cursor); }); }; + // -------------------------------------------------------------------------// + // Replace multiple interface functions. This is required to fully support // + // previews without changing readlines behavior. // + // -------------------------------------------------------------------------// + // Refresh prints the whole screen again and the preview will be removed // during that procedure. Print the preview again. This also makes sure // the preview is always correct after resizing the terminal window. - const tmpRefresh = repl._refreshLine.bind(repl); + const originalRefresh = repl._refreshLine.bind(repl); repl._refreshLine = () => { - preview = null; - tmpRefresh(); - showInputPreview(); + inputPreview = null; + originalRefresh(); + showPreview(); + }; + + let insertCompletionPreview = true; + // Insert the longest common suffix of the current input in case the user + // moves to the right while already being at the current input end. + const originalMoveCursor = repl._moveCursor.bind(repl); + repl._moveCursor = (dx) => { + const currentCursor = repl.cursor; + originalMoveCursor(dx); + if (currentCursor + dx > repl.line.length && + typeof repl.completer === 'function' && + insertCompletionPreview) { + const insertPreview = true; + showCompletionPreview(repl.line, insertPreview); + } + }; + + // This is the only function that interferes with the completion insertion. + // Monkey patch it to prevent inserting the completion when it shouldn't be. + const originalClearLine = repl.clearLine.bind(repl); + repl.clearLine = () => { + insertCompletionPreview = false; + originalClearLine(); + insertCompletionPreview = true; }; - return { showInputPreview, clearPreview }; + return { showPreview, clearPreview }; } module.exports = { diff --git a/lib/readline.js b/lib/readline.js index d471d512f485d5..daf30969408222 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -601,8 +601,11 @@ function charLengthLeft(str, i) { } function charLengthAt(str, i) { - if (str.length <= i) - return 0; + 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; } @@ -956,6 +959,7 @@ Interface.prototype._ttyWrite = function(s, key) { } break; + // TODO(BridgeAR): This seems broken? case 'w': // Delete backwards to a word boundary case 'backspace': this._deleteWordLeft(); diff --git a/lib/repl.js b/lib/repl.js index cd8240dd743919..24948477fea65e 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -810,7 +810,7 @@ function REPLServer(prompt, const { clearPreview, - showInputPreview + showPreview } = setupPreview( this, kContextId, @@ -821,7 +821,6 @@ function REPLServer(prompt, // Wrap readline tty to enable editor mode and pausing. const ttyWrite = self._ttyWrite.bind(self); self._ttyWrite = (d, key) => { - clearPreview(); key = key || {}; if (paused && !(self.breakEvalOnSigint && key.ctrl && key.name === 'c')) { pausedBuffer.push(['key', [d, key]]); @@ -833,14 +832,17 @@ function REPLServer(prompt, self.cursor === 0 && self.line.length === 0) { self.clearLine(); } + clearPreview(); ttyWrite(d, key); - showInputPreview(); + showPreview(); return; } // Editor mode if (key.ctrl && !key.shift) { switch (key.name) { + // TODO(BridgeAR): There should not be a special mode necessary for full + // multiline support. case 'd': // End editor mode _turnOffEditorMode(self); sawCtrlD = true; diff --git a/test/parallel/test-repl-editor.js b/test/parallel/test-repl-editor.js index 969f6172b3fb70..b340bd66313971 100644 --- a/test/parallel/test-repl-editor.js +++ b/test/parallel/test-repl-editor.js @@ -5,10 +5,11 @@ const assert = require('assert'); const repl = require('repl'); const ArrayStream = require('../common/arraystream'); -// \u001b[1G - Moves the cursor to 1st column +// \u001b[nG - Moves the cursor to n st column // \u001b[0J - Clear screen -// \u001b[3G - Moves the cursor to 3rd column +// \u001b[0K - Clear to line end const terminalCode = '\u001b[1G\u001b[0J> \u001b[3G'; +const previewCode = (str, n) => ` // ${str}\x1B[${n}G\x1B[0K`; const terminalCodeRegex = new RegExp(terminalCode.replace(/\[/g, '\\['), 'g'); function run({ input, output, event, checkTerminalCodes = true }) { @@ -17,7 +18,9 @@ function run({ input, output, event, checkTerminalCodes = true }) { stream.write = (msg) => found += msg.replace('\r', ''); - let expected = `${terminalCode}.editor\n` + + let expected = `${terminalCode}.ed${previewCode('itor', 6)}i` + + `${previewCode('tor', 7)}t${previewCode('or', 8)}o` + + `${previewCode('r', 9)}r\n` + '// Entering editor mode (^D to finish, ^C to cancel)\n' + `${input}${output}\n${terminalCode}`; diff --git a/test/parallel/test-repl-multiline.js b/test/parallel/test-repl-multiline.js index 6498923b62ecfc..f99b91c84b0a85 100644 --- a/test/parallel/test-repl-multiline.js +++ b/test/parallel/test-repl-multiline.js @@ -23,14 +23,23 @@ function run({ useColors }) { r.on('exit', common.mustCall(() => { const actual = output.split('\n'); + const firstLine = useColors ? + '\x1B[1G\x1B[0J \x1B[1Gco\x1B[90mn\x1B[39m\x1B[3G\x1B[0Knst ' + + 'fo\x1B[90mr\x1B[39m\x1B[9G\x1B[0Ko = {' : + '\x1B[1G\x1B[0J \x1B[1Gco // n\x1B[3G\x1B[0Knst ' + + 'fo // r\x1B[9G\x1B[0Ko = {'; + // Validate the output, which contains terminal escape codes. assert.strictEqual(actual.length, 6 + process.features.inspector); - assert.ok(actual[0].endsWith(input[0])); + assert.strictEqual(actual[0], firstLine); assert.ok(actual[1].includes('... ')); assert.ok(actual[1].endsWith(input[1])); assert.ok(actual[2].includes('undefined')); - assert.ok(actual[3].endsWith(input[2])); if (process.features.inspector) { + assert.ok( + actual[3].endsWith(input[2]), + `"${actual[3]}" should end with "${input[2]}"` + ); assert.ok(actual[4].includes(actual[5])); assert.strictEqual(actual[4].includes('//'), !useColors); } diff --git a/test/parallel/test-repl-preview.js b/test/parallel/test-repl-preview.js index 92e73dd245056f..65b56904cb03b9 100644 --- a/test/parallel/test-repl-preview.js +++ b/test/parallel/test-repl-preview.js @@ -72,13 +72,13 @@ async function tests(options) { '\x1B[36m[Function: foo]\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['koo', [2, 4], '[Function: koo]', - 'koo', + 'k\x1B[90moo\x1B[39m\x1B[9G\x1B[0Ko\x1B[90mo\x1B[39m\x1B[10G\x1B[0Ko', '\x1B[90m[Function: koo]\x1B[39m\x1B[1A\x1B[11G\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[36m[Function: koo]\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['a', [1, 2], undefined], ['{ a: true }', [2, 3], '{ a: \x1B[33mtrue\x1B[39m }', - '{ a: true }\r', + '{ a: tru\x1B[90me\x1B[39m\x1B[16G\x1B[0Ke }\r', '{ a: \x1B[33mtrue\x1B[39m }', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['1n + 2n', [2, 5], '\x1B[33m3n\x1B[39m', @@ -88,12 +88,12 @@ async function tests(options) { '\x1B[33m3n\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], ['{ a: true };', [2, 4], '\x1B[33mtrue\x1B[39m', - '{ a: true };', + '{ a: tru\x1B[90me\x1B[39m\x1B[16G\x1B[0Ke };', '\x1B[90mtrue\x1B[39m\x1B[1A\x1B[20G\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[33mtrue\x1B[39m', '\x1B[1G\x1B[0Jrepl > \x1B[8G'], [' \t { a: true};', [2, 5], '\x1B[33mtrue\x1B[39m', - ' \t { a: true}', + ' \t { a: tru\x1B[90me\x1B[39m\x1B[19G\x1B[0Ke}', '\x1B[90m{ a: true }\x1B[39m\x1B[1A\x1B[21G\x1B[1B\x1B[2K\x1B[1A;', '\x1B[90mtrue\x1B[39m\x1B[1A\x1B[22G\x1B[1B\x1B[2K\x1B[1A\r', '\x1B[33mtrue\x1B[39m', diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js index cecbd3ab4563d0..47fcb8530dee77 100644 --- a/test/parallel/test-repl-top-level-await.js +++ b/test/parallel/test-repl-top-level-await.js @@ -60,7 +60,7 @@ const testMe = repl.start({ prompt: PROMPT, stream: putIn, terminal: true, - useColors: false, + useColors: true, breakEvalOnSigint: true }); @@ -84,69 +84,99 @@ async function ordinaryTests() { 'function koo() { return Promise.resolve(4); }' ]); const testCases = [ - [ 'await Promise.resolve(0)', '0' ], - [ '{ a: await Promise.resolve(1) }', '{ a: 1 }' ], - [ '_', '// { a: 1 }\r', { line: 0 } ], + [ 'await Promise.resolve(0)', + // Auto completion preview with colors stripped. + ['awaitaititt Proroomiseisesee.resolveolvelvevee(0)\r', '0'] + ], + [ '{ a: await Promise.resolve(1) }', + // Auto completion preview with colors stripped. + ['{ a: awaitaititt Proroomiseisesee.resolveolvelvevee(1) }\r', + '{ a: 1 }'] + ], + [ '_', '{ a: 1 }\r', { line: 0 } ], [ 'let { aa, bb } = await Promise.resolve({ aa: 1, bb: 2 }), f = 5;', - 'undefined' ], - [ 'aa', ['// 1\r', '1'] ], - [ 'bb', ['// 2\r', '2'] ], - [ 'f', ['// 5\r', '5'] ], - [ 'let cc = await Promise.resolve(2)', 'undefined' ], - [ 'cc', ['// 2\r', '2'] ], - [ 'let dd;', 'undefined' ], - [ 'dd', 'undefined' ], - [ 'let [ii, { abc: { kk } }] = [0, { abc: { kk: 1 } }];', 'undefined' ], - [ 'ii', ['// 0\r', '0'] ], - [ 'kk', ['// 1\r', '1'] ], - [ 'var ll = await Promise.resolve(2);', 'undefined' ], - [ 'll', ['// 2\r', '2'] ], + [ + 'letett { aa, bb } = awaitaititt Proroomiseisesee.resolveolvelvevee' + + '({ aa: 1, bb: 2 }), f = 5;\r' + ] + ], + [ 'aa', ['1\r', '1'] ], + [ 'bb', ['2\r', '2'] ], + [ 'f', ['5\r', '5'] ], + [ 'let cc = await Promise.resolve(2)', + ['letett cc = awaitaititt Proroomiseisesee.resolveolvelvevee(2)\r'] + ], + [ 'cc', ['2\r', '2'] ], + [ 'let dd;', ['letett dd;\r'] ], + [ 'dd', ['undefined\r'] ], + [ 'let [ii, { abc: { kk } }] = [0, { abc: { kk: 1 } }];', + ['letett [ii, { abc: { kook } }] = [0, { abc: { kook: 1 } }];\r'] ], + [ 'ii', ['0\r', '0'] ], + [ 'kk', ['1\r', '1'] ], + [ 'var ll = await Promise.resolve(2);', + ['var letl = awaitaititt Proroomiseisesee.resolveolvelvevee(2);\r'] + ], + [ 'll', ['2\r', '2'] ], [ 'foo(await koo())', - [ 'f', '// 5oo', '// [Function: foo](await koo())\r', '4' ] ], - [ '_', ['// 4\r', '4'] ], + ['f', '5oo', '[Function: foo](awaitaititt kooo())\r', '4'] ], + [ '_', ['4\r', '4'] ], [ 'const m = foo(await koo());', - [ 'const m = foo(await koo());\r', 'undefined' ] ], - [ 'm', ['// 4\r', '4' ] ], + ['connst module = foo(awaitaititt kooo());\r'] ], + [ 'm', ['4\r', '4' ] ], [ 'const n = foo(await\nkoo());', - [ 'const n = foo(await\r', '... koo());\r', 'undefined' ] ], - [ 'n', ['// 4\r', '4' ] ], + ['connst n = foo(awaitaititt\r', '... kooo());\r', 'undefined'] ], + [ 'n', ['4\r', '4'] ], // eslint-disable-next-line no-template-curly-in-string [ '`status: ${(await Promise.resolve({ status: 200 })).status}`', - "'status: 200'"], + [ + '`stratus: ${(awaitaititt Proroomiseisesee.resolveolvelvevee' + + '({ stratus: 200 })).stratus}`\r', + "'status: 200'" + ] + ], [ 'for (let i = 0; i < 2; ++i) await i', - ['f', '// 5or (let i = 0; i < 2; ++i) await i\r', 'undefined'] ], + ['f', '5or (lett i = 0; i < 2; ++i) awaitaititt i\r', 'undefined'] ], [ 'for (let i = 0; i < 2; ++i) { await i }', - [ 'f', '// 5or (let i = 0; i < 2; ++i) { await i }\r', 'undefined' ] ], - [ 'await 0', ['await 0\r', '0'] ], + ['f', '5or (lett i = 0; i < 2; ++i) { awaitaititt i }\r', 'undefined'] + ], + [ 'await 0', ['awaitaititt 0\r', '0'] ], [ 'await 0; function foo() {}', - ['await 0; function foo() {}\r', 'undefined'] ], + ['awaitaititt 0; functionnctionctiontioniononn foo() {}\r'] + ], [ 'foo', - ['f', '// 5oo', '// [Function: foo]\r', '[Function: foo]'] ], - [ 'class Foo {}; await 1;', ['class Foo {}; await 1;\r', '1'] ], - [ 'Foo', ['// [Function: Foo]\r', '[Function: Foo]'] ], + ['f', '5oo', '[Function: foo]\r', '[Function: foo]'] ], + [ 'class Foo {}; await 1;', ['class Foo {}; awaitaititt 1;\r', '1'] ], + [ 'Foo', ['Fooo', '[Function: Foo]\r', '[Function: Foo]'] ], [ 'if (await true) { function bar() {}; }', - ['if (await true) { function bar() {}; }\r', 'undefined'] ], - [ 'bar', ['// [Function: bar]\r', '[Function: bar]'] ], - [ 'if (await true) { class Bar {}; }', 'undefined' ], + ['if (awaitaititt truee) { functionnctionctiontioniononn bar() {}; }\r'] + ], + [ 'bar', ['barr', '[Function: bar]\r', '[Function: bar]'] ], + [ 'if (await true) { class Bar {}; }', + ['if (awaitaititt truee) { class Bar {}; }\r'] + ], [ 'Bar', 'Uncaught ReferenceError: Bar is not defined' ], - [ 'await 0; function* gen(){}', 'undefined' ], + [ 'await 0; function* gen(){}', + ['awaitaititt 0; functionnctionctiontioniononn* globalen(){}\r'] + ], [ 'for (var i = 0; i < 10; ++i) { await i; }', - ['f', '// 5or (var i = 0; i < 10; ++i) { await i; }\r', 'undefined'] ], - [ 'i', ['// 10\r', '10'] ], + ['f', '5or (var i = 0; i < 10; ++i) { awaitaititt i; }\r', 'undefined'] ], + [ 'i', ['10\r', '10'] ], [ 'for (let j = 0; j < 5; ++j) { await j; }', - ['f', '// 5or (let j = 0; j < 5; ++j) { await j; }\r', 'undefined'] ], + ['f', '5or (lett j = 0; j < 5; ++j) { awaitaititt j; }\r', 'undefined'] ], [ 'j', 'Uncaught ReferenceError: j is not defined', { line: 0 } ], - [ 'gen', ['// [GeneratorFunction: gen]\r', '[GeneratorFunction: gen]'] ], + [ 'gen', + ['genn', '[GeneratorFunction: gen]\r', '[GeneratorFunction: gen]'] + ], [ 'return 42; await 5;', 'Uncaught SyntaxError: Illegal return statement', { line: 3 } ], - [ 'let o = await 1, p', 'undefined' ], - [ 'p', 'undefined' ], - [ 'let q = 1, s = await 2', 'undefined' ], - [ 's', ['// 2\r', '2'] ], + [ 'let o = await 1, p', ['lett os = awaitaititt 1, p\r'] ], + [ 'p', ['undefined\r'] ], + [ 'let q = 1, s = await 2', ['lett que = 1, s = awaitaititt 2\r'] ], + [ 's', ['2\r', '2'] ], [ 'for await (let i of [1,2,3]) console.log(i)', [ 'f', - '// 5or await (let i of [1,2,3]) console.log(i)\r', + '5or awaitaititt (lett i of [1,2,3]) connsolelee.logogg(i)\r', '1', '2', '3', @@ -160,6 +190,8 @@ async function ordinaryTests() { const toBeRun = input.split('\n'); const lines = await runAndWait(toBeRun); if (Array.isArray(expected)) { + if (expected.length === 1) + expected.push('undefined'); if (lines[0] === input) lines.shift(); assert.deepStrictEqual(lines, [...expected, PROMPT]); @@ -184,7 +216,7 @@ async function ctrlCTest() { 'await timeout(100000)', { ctrl: true, name: 'c' } ]), [ - 'await timeout(100000)\r', + 'awaitaititt timeoutmeouteoutoututt(100000)\r', 'Uncaught:', '[Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' + 'Script execution was interrupted by `SIGINT`] {', From 707cba4493ddafa16723e33cf139eb934680a1a3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 11 Dec 2019 19:37:44 +0100 Subject: [PATCH 09/12] repl: fix preview bug in case of long lines This addresses an issue that is caused by lines that exceed the current window columns. That would cause the preview to confuse the REPL. This is meant as hot fix. The preview should be able to handle these cases appropriately as well later on. --- lib/internal/repl/utils.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index 3deee9a57105b2..766e655a44fafa 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -275,6 +275,16 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) { return; } + // Do not show previews in case the current line is longer than the column + // width. + // TODO(BridgeAR): Fix me. This should not be necessary. It currently breaks + // the output though. We also have to check for characters that have more + // than a single byte as length. Check Interface.prototype._moveCursor. It + // contains the necessary logic. + if (repl.line.length + repl._prompt.length > repl.columns) { + return; + } + // Add the autocompletion preview. // TODO(BridgeAR): Trigger the input preview after the completion preview. // That way it's possible to trigger the input prefix including the From ca4c894efe5488d21ccd4076479eca204eb413f0 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 13 Dec 2019 02:44:32 +0100 Subject: [PATCH 10/12] test: add multiple repl preview tests This improves the coverage for the preview feature signficantly. Quite a few edge cases get testet here to prevent regressions. --- test/parallel/test-repl-history-navigation.js | 210 +++++++++++++++++- test/parallel/test-repl-preview.js | 4 +- test/root.status | 1 + 3 files changed, 202 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index 3bd198880fc015..f66a27ff83b715 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -8,6 +8,7 @@ const REPL = require('internal/repl'); const assert = require('assert'); const fs = require('fs'); const path = require('path'); +const { inspect } = require('util'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -17,6 +18,7 @@ const defaultHistoryPath = path.join(tmpdir.path, '.node_repl_history'); // Create an input stream specialized for testing an array of actions class ActionStream extends stream.Stream { run(data) { + let reallyWait = true; const _iter = data[Symbol.iterator](); const doAction = () => { const next = _iter.next(); @@ -32,24 +34,34 @@ class ActionStream extends stream.Stream { } else { this.emit('data', `${action}`); } - setImmediate(doAction); + if (action === WAIT && reallyWait) { + setTimeout(doAction, common.platformTimeout(50)); + reallyWait = false; + } else { + setImmediate(doAction); + } }; - setImmediate(doAction); + doAction(); } resume() {} pause() {} } ActionStream.prototype.readable = true; - // Mock keys const ENTER = { name: 'enter' }; const UP = { name: 'up' }; const DOWN = { name: 'down' }; const LEFT = { name: 'left' }; +const RIGHT = { name: 'right' }; const DELETE = { name: 'delete' }; +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 prompt = '> '; +const WAIT = '€'; const prev = process.features.inspector; @@ -91,6 +103,158 @@ const tests = [ ' 2025, 2116, 2209, ...', prompt].filter((e) => typeof e === 'string'), clean: true + }, + { + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + skip: !process.features.inspector, + test: [ + `const ${'veryLongName'.repeat(30)} = 'I should not be previewed'`, + ENTER, + 'const e = new RangeError("visible\\ninvisible")', + ENTER, + 'e', + ENTER, + 'veryLongName'.repeat(30), + ENTER, + `${'\u001b[90m \u001b[39m'.repeat(230)} fun`, + ENTER, + `${' '.repeat(245)} fun`, + ENTER + ], + expected: [], + clean: false + }, + { + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + columns: 250, + skip: !process.features.inspector, + test: [ + UP, + UP, + UP, + UP, + BACKSPACE + ], + expected: [ + prompt, + `${prompt}${' '.repeat(245)} fun`, + `${prompt}${' '.repeat(230)} fun`, + ' // ction', + ' // ction', + `${prompt}${'veryLongName'.repeat(30)}`, + `${prompt}e`, + '\n// RangeError: visible' + ], + clean: true + }, + { + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + showEscapeCodes: true, + skip: !process.features.inspector, + test: [ + 'fun', + RIGHT, + BACKSPACE, + LEFT, + LEFT, + 'A', + BACKSPACE, + GO_TO_END, + BACKSPACE, + WORD_LEFT, + WORD_RIGHT, + ENTER + ], + // C = Cursor forward + // D = Cursor back + // G = Cursor to column n + // J = Erase in screen + // K = Erase in line + expected: [ + // 0. + // 'f' + '\x1B[1G', '\x1B[0J', prompt, '\x1B[3G', 'f', + // 'u' + 'u', ' // nction', '\x1B[5G', + // 'n' - Cleanup + '\x1B[0K', + 'n', ' // ction', '\x1B[6G', + // 1. Right. Cleanup + '\x1B[0K', + 'ction', + // 2. Backspace. Refresh + '\x1B[1G', '\x1B[0J', `${prompt}functio`, '\x1B[10G', + // Autocomplete and refresh? + ' // n', '\x1B[10G', ' // n', '\x1B[10G', + // 3. Left. Cleanup + '\x1B[0K', + '\x1B[1D', '\x1B[10G', ' // n', '\x1B[9G', + // 4. Left. Cleanup + '\x1B[10G', '\x1B[0K', '\x1B[9G', + '\x1B[1D', '\x1B[10G', ' // n', '\x1B[8G', + // 5. 'A' - Cleanup + '\x1B[10G', '\x1B[0K', '\x1B[8G', + // Refresh + '\x1B[1G', '\x1B[0J', `${prompt}functAio`, '\x1B[9G', + // 6. Backspace. Refresh + '\x1B[1G', '\x1B[0J', `${prompt}functio`, '\x1B[8G', '\x1B[10G', ' // n', + '\x1B[8G', '\x1B[10G', ' // n', + '\x1B[8G', '\x1B[10G', + // 7. Go to end. Cleanup + '\x1B[0K', '\x1B[8G', '\x1B[2C', + 'n', + // 8. Backspace. Refresh + '\x1B[1G', '\x1B[0J', `${prompt}functio`, '\x1B[10G', + // Autocomplete + ' // n', '\x1B[10G', ' // n', '\x1B[10G', + // 9. Word left. Cleanup + '\x1B[0K', '\x1B[7D', '\x1B[10G', ' // n', '\x1B[3G', '\x1B[10G', + // 10. Word right. Cleanup + '\x1B[0K', '\x1B[3G', '\x1B[7C', ' // n', '\x1B[10G', + '\x1B[0K', + ], + clean: true + }, + { + // Check that the completer ignores completions that are outdated. + env: { NODE_REPL_HISTORY: defaultHistoryPath }, + completer(line, callback) { + if (line.endsWith(WAIT)) { + setTimeout( + callback, + common.platformTimeout(40), + null, + [[`${WAIT}WOW`], line] + ); + } else { + callback(null, [[' Always visible'], line]); + } + }, + skip: !process.features.inspector, + test: [ + WAIT, // The first call is awaited before new input is triggered! + BACKSPACE, + 's', + BACKSPACE, + WAIT, // The second call is not awaited. It won't trigger the preview. + BACKSPACE, + 's', + BACKSPACE + ], + expected: [ + prompt, + WAIT, + ' // WOW', + prompt, + 's', + ' // Always visible', + prompt, + WAIT, + prompt, + 's', + ' // Always visible', + ], + clean: true } ]; const numtests = tests.length; @@ -112,29 +276,47 @@ function runTest() { const opts = tests.shift(); if (!opts) return; // All done - const env = opts.env; - const test = opts.test; - const expected = opts.expected; + const { expected, skip } = opts; + + // Test unsupported on platform. + if (skip) { + setImmediate(runTestWrap, true); + return; + } + + const lastChunks = []; - REPL.createInternalRepl(env, { + REPL.createInternalRepl(opts.env, { input: new ActionStream(), output: new stream.Writable({ write(chunk, _, next) { const output = chunk.toString(); - if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output)) { + if (!opts.showEscapeCodes && + output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output)) { return next(); } + lastChunks.push(inspect(output)); + if (expected.length) { - assert.strictEqual(output, expected[0]); + try { + assert.strictEqual(output, expected[0]); + } catch (e) { + console.error(`Failed test # ${numtests - tests.length}`); + console.error('Last outputs: ' + inspect(lastChunks, { + breakLength: 5, colors: true + })); + throw e; + } expected.shift(); } next(); } }), - prompt: prompt, + completer: opts.completer, + prompt, useColors: false, terminal: true }, function(err, repl) { @@ -153,7 +335,13 @@ function runTest() { setImmediate(runTestWrap, true); }); - repl.inputStream.run(test); + if (opts.columns) { + Object.defineProperty(repl, 'columns', { + value: opts.columns, + enumerable: true + }); + } + repl.inputStream.run(opts.test); }); } diff --git a/test/parallel/test-repl-preview.js b/test/parallel/test-repl-preview.js index 65b56904cb03b9..cd34c461d80671 100644 --- a/test/parallel/test-repl-preview.js +++ b/test/parallel/test-repl-preview.js @@ -3,7 +3,7 @@ const common = require('../common'); const ArrayStream = require('../common/arraystream'); const assert = require('assert'); -const Repl = require('repl'); +const { REPLServer } = require('repl'); common.skipIfInspectorDisabled(); @@ -52,7 +52,7 @@ function runAndWait(cmds, repl) { } async function tests(options) { - const repl = Repl.start({ + const repl = REPLServer({ prompt: PROMPT, stream: new REPLStream(), ignoreUndefined: true, diff --git a/test/root.status b/test/root.status index 91aad08caa3527..6edb9ddec34ca2 100644 --- a/test/root.status +++ b/test/root.status @@ -66,6 +66,7 @@ parallel/test-next-tick-fixed-queue-regression: SLOW parallel/test-npm-install: SLOW parallel/test-preload: SLOW parallel/test-repl: SLOW +parallel/test-repl-history-navigation.js: SLOW parallel/test-repl-tab-complete: SLOW parallel/test-repl-top-level-await: SLOW parallel/test-stdio-pipe-access: SLOW From 0cd4153343ee8d987ba77de5ce0bd2318d3480ff Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 14 Dec 2019 20:27:01 +0100 Subject: [PATCH 11/12] fixup: repl: add completion preview --- lib/internal/repl/utils.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/internal/repl/utils.js b/lib/internal/repl/utils.js index 766e655a44fafa..c54e173bdf3e1c 100644 --- a/lib/internal/repl/utils.js +++ b/lib/internal/repl/utils.js @@ -188,9 +188,14 @@ 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 (repl.line.length + repl._prompt.length + suffix > repl.columns) { + if (totalLength > repl.columns) { return; } From 2138fd790661d284ba8de799b4c10f27c6092beb Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 14 Dec 2019 20:27:08 +0100 Subject: [PATCH 12/12] fixup: test: add multiple repl preview tests --- test/parallel/test-repl-history-navigation.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-repl-history-navigation.js b/test/parallel/test-repl-history-navigation.js index f66a27ff83b715..f73fbb9b0fd278 100644 --- a/test/parallel/test-repl-history-navigation.js +++ b/test/parallel/test-repl-history-navigation.js @@ -116,9 +116,9 @@ const tests = [ ENTER, 'veryLongName'.repeat(30), ENTER, - `${'\u001b[90m \u001b[39m'.repeat(230)} fun`, + `${'\x1B[90m \x1B[39m'.repeat(235)} fun`, ENTER, - `${' '.repeat(245)} fun`, + `${' '.repeat(236)} fun`, ENTER ], expected: [], @@ -137,13 +137,17 @@ const tests = [ ], expected: [ prompt, - `${prompt}${' '.repeat(245)} fun`, - `${prompt}${' '.repeat(230)} fun`, + // This exceeds the maximum columns (250): + // Whitespace + prompt + ' // '.length + 'function'.length + // 236 + 2 + 4 + 8 + `${prompt}${' '.repeat(236)} fun`, + `${prompt}${' '.repeat(235)} fun`, ' // ction', ' // ction', `${prompt}${'veryLongName'.repeat(30)}`, `${prompt}e`, - '\n// RangeError: visible' + '\n// RangeError: visible', + prompt ], clean: true },