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. PR-URL: #25731 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs · Feb 6, 2019 · e66cb58 · e66cb58
1 parent 1c6fade
commit e66cb58
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 42 deletions.
diff --git a/lib/repl.js b/lib/repl.js
@@ -90,18 +90,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');
 
@@ -806,24 +794,17 @@ 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', {
       configurable: true,
       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('<repl>');
@@ -1135,19 +1116,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();
           });
@@ -1371,21 +1352,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
@@ -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) {