From 9c668c219994d0ea4004dbd9d07a37015158f793 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 25 Jan 2019 08:26:53 +0100 Subject: [PATCH 1/4] repl: simplify and improve completion The completion lists used a hand crafted list of global entries that was redundant due to also using the actual global properties for tab completion. Those entries ended up in an separated completion group which did not seem useful. --- lib/repl.js | 58 ++++++++----------------- test/parallel/test-repl-tab-complete.js | 2 +- 2 files changed, 18 insertions(+), 42 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 30812c232b7639..df861843d1d501 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -89,18 +89,6 @@ let processTopLevelAwait; const parentModule = module; const replMap = new WeakMap(); -const GLOBAL_OBJECT_PROPERTIES = [ - 'NaN', 'Infinity', 'undefined', 'eval', 'parseInt', 'parseFloat', 'isNaN', - 'isFinite', 'decodeURI', 'decodeURIComponent', 'encodeURI', - 'encodeURIComponent', 'Object', 'Function', 'Array', 'String', 'Boolean', - 'Number', 'Date', 'RegExp', 'Error', 'EvalError', 'RangeError', - 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', 'Math', 'JSON' -]; -const GLOBAL_OBJECT_PROPERTY_MAP = {}; -for (var n = 0; n < GLOBAL_OBJECT_PROPERTIES.length; n++) { - GLOBAL_OBJECT_PROPERTY_MAP[GLOBAL_OBJECT_PROPERTIES[n]] = - GLOBAL_OBJECT_PROPERTIES[n]; -} const kBufferedCommandSymbol = Symbol('bufferedCommand'); const kContextId = Symbol('contextId'); @@ -807,6 +795,10 @@ REPLServer.prototype.createContext = function() { }, () => { context = vm.createContext(); }); + for (const name of Object.getOwnPropertyNames(global)) { + Object.defineProperty(context, name, + Object.getOwnPropertyDescriptor(global, name)); + } context.global = context; const _console = new Console(this.outputStream); Object.defineProperty(context, 'console', { @@ -814,17 +806,6 @@ REPLServer.prototype.createContext = function() { writable: true, value: _console }); - - var names = Object.getOwnPropertyNames(global); - for (var n = 0; n < names.length; n++) { - var name = names[n]; - if (name === 'console' || name === 'global') - continue; - if (GLOBAL_OBJECT_PROPERTY_MAP[name] === undefined) { - Object.defineProperty(context, name, - Object.getOwnPropertyDescriptor(global, name)); - } - } } var module = new CJSModule(''); @@ -1137,19 +1118,19 @@ function complete(line, callback) { } completionGroups.push( filteredOwnPropertyNames.call(this, this.context)); - addStandardGlobals(completionGroups, filter); + if (filter !== '') addCommonWords(completionGroups); completionGroupsLoaded(); } else { this.eval('.scope', this.context, 'repl', function ev(err, globals) { if (err || !Array.isArray(globals)) { - addStandardGlobals(completionGroups, filter); + if (filter !== '') addCommonWords(completionGroups); } else if (Array.isArray(globals[0])) { // Add grouped globals for (var n = 0; n < globals.length; n++) completionGroups.push(globals[n]); } else { completionGroups.push(globals); - addStandardGlobals(completionGroups, filter); + if (filter !== '') addCommonWords(completionGroups); } completionGroupsLoaded(); }); @@ -1373,21 +1354,16 @@ function _memory(cmd) { } } -function addStandardGlobals(completionGroups, filter) { - // Global object properties - // (http://www.ecma-international.org/publications/standards/Ecma-262.htm) - completionGroups.push(GLOBAL_OBJECT_PROPERTIES); - // Common keywords. Exclude for completion on the empty string, b/c - // they just get in the way. - if (filter) { - completionGroups.push([ - 'async', 'await', 'break', 'case', 'catch', 'const', 'continue', - 'debugger', 'default', 'delete', 'do', 'else', 'export', 'false', - 'finally', 'for', 'function', 'if', 'import', 'in', 'instanceof', 'let', - 'new', 'null', 'return', 'switch', 'this', 'throw', 'true', 'try', - 'typeof', 'undefined', 'var', 'void', 'while', 'with', 'yield' - ]); - } +function addCommonWords(completionGroups) { + // Only words which do not yet exist as global property should be added to + // this list. + completionGroups.push([ + 'async', 'await', 'break', 'case', 'catch', 'const', 'continue', + 'debugger', 'default', 'delete', 'do', 'else', 'export', 'false', + 'finally', 'for', 'function', 'if', 'import', 'in', 'instanceof', 'let', + 'new', 'null', 'return', 'switch', 'this', 'throw', 'true', 'try', + 'typeof', 'var', 'void', 'while', 'with', 'yield' + ]); } function _turnOnEditorMode(repl) { diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 69011e4af894f2..0325678ff2e200 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -457,7 +457,7 @@ const testNonGlobal = repl.start({ useGlobal: false }); -const builtins = [['Infinity', '', 'Int16Array', 'Int32Array', +const builtins = [['Infinity', 'Int16Array', 'Int32Array', 'Int8Array'], 'I']; if (common.hasIntl) { From 0542890e4d42c591912512928312bfbc4e0aa4bc Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 27 Jan 2019 01:18:47 +0100 Subject: [PATCH 2/4] repl: fix eval return value In case no error has occurred during the evaluation of some code, `undefined` has been returned in some cases as error argument instead of `null`. This is fixed by this patch. --- lib/repl.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index df861843d1d501..e0f5373c8c2559 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -215,10 +215,11 @@ function REPLServer(prompt, } function defaultEval(code, context, file, cb) { - var err, result, script, wrappedErr; - var wrappedCmd = false; - var awaitPromise = false; - var input = code; + let result, script, wrappedErr; + let err = null; + let wrappedCmd = false; + let awaitPromise = false; + const input = code; if (/^\s*\{/.test(code) && /\}\s*$/.test(code)) { // It's confusing for `{ a : 1 }` to be interpreted as a block @@ -362,7 +363,7 @@ function REPLServer(prompt, } promise.then((result) => { - finishExecution(undefined, result); + finishExecution(null, result); }, (err) => { if (err && process.domain) { debug('not recoverable, send to domain'); From 240388c2028e0b77065858171e06cf9b9ad5b61d Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 27 Jan 2019 01:20:08 +0100 Subject: [PATCH 3/4] repl: remove obsolete buffer clearing It is checked if an command buffer exists or not. This code branch can only be reached if non exist, so there's no need to clear that buffer again. --- lib/repl.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index e0f5373c8c2559..25680169486701 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -637,7 +637,6 @@ function REPLServer(prompt, self.outputStream.write('npm should be run outside of the ' + 'node repl, in your normal shell.\n' + '(Press Control-D to exit.)\n'); - self.clearBufferedCommand(); self.displayPrompt(); return; } From 12a3e4d0bdbc5ab0cf715dbebdf129d117315df3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 27 Jan 2019 01:59:24 +0100 Subject: [PATCH 4/4] test: remove obsolete code The removed line does not add anything of value to the test. It was removed to simplify the test. --- test/parallel/test-repl-tab-complete-no-warn.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-repl-tab-complete-no-warn.js b/test/parallel/test-repl-tab-complete-no-warn.js index 58d0a1332fb193..7aedee69c31cb7 100644 --- a/test/parallel/test-repl-tab-complete-no-warn.js +++ b/test/parallel/test-repl-tab-complete-no-warn.js @@ -15,7 +15,6 @@ const testMe = repl.start('', putIn); // `Runtime.executionContextCreated` listener process.on('warning', common.mustNotCall()); -putIn.run(['.clear']); putIn.run(['async function test() {']); for (let i = 0; i < DEFAULT_MAX_LISTENERS; i++) { testMe.complete('await Promise.resolve()', () => {});