DevTools: sort completions by prototype.

third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js

Index: third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js
diff --git a/third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js b/third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js
index 6a71a77674a418f342dfa1d774c2ceabf54d0f0f..2b7da7b2b798c25507a9845328ae743c51f49b6b 100644
--- a/third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js
+++ b/third_party/WebKit/Source/devtools/front_end/components/JavaScriptAutocomplete.js
@@ -4,6 +4,9 @@
 
 Components.JavaScriptAutocomplete = {};
 
+/** @typedef {{title:(string|undefined), items:Array<string>}} */
+Components.JavaScriptAutocomplete.CompletionGroup;
+
 /**
  * @param {string} text
  * @param {string} query
@@ -39,13 +42,12 @@ Components.JavaScriptAutocomplete.completionsForTextInCurrentContext = function(
   return Components.JavaScriptAutocomplete.completionsForExpression(clippedExpression, query, force);
 };
 
-
 /**
-   * @param {string} expressionString
-   * @param {string} query
-   * @param {boolean=} force
-   * @return {!Promise<!UI.SuggestBox.Suggestions>}
-   */
+ * @param {string} expressionString
+ * @param {string} query
+ * @param {boolean=} force
+ * @return {!Promise<!UI.SuggestBox.Suggestions>}
+ */
 Components.JavaScriptAutocomplete.completionsForExpression = function(expressionString, query, force) {
   var executionContext = UI.context.flavor(SDK.ExecutionContext);
   if (!executionContext)
@@ -71,7 +73,7 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
   var fufill;
   var promise = new Promise(x => fufill = x);
   if (!expressionString && executionContext.debuggerModel.selectedCallFrame())
-    executionContext.debuggerModel.selectedCallFrame().variableNames(receivedPropertyNames);
+    variableNamesInScopes(executionContext.debuggerModel.selectedCallFrame(), receivedPropertyNames);
   else
     executionContext.evaluate(expressionString, 'completion', true, true, false, false, false, evaluated);
 
@@ -125,23 +127,32 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
       else
         object = this;
 
-      var resultSet = {__proto__: null};
+      var result = [];
       try {
         for (var o = object; o; o = Object.getPrototypeOf(o)) {
           if ((type === 'array' || type === 'typedarray') && o === object && ArrayBuffer.isView(o) && o.length > 9999)
             continue;
+
+          var group = {items: [], __proto__: null};
+          try {
+            if (typeof o === 'object' && o.constructor && o.constructor.name)

[einbinder, 2016/12/07 20:10:50]

//TODO (einbinder) namespaces?

+              group.title = o.constructor.name;
+          } catch (ee) {
+            // we could break upon cross origin check.
+          }
+          result[result.length] = group;
           var names = Object.getOwnPropertyNames(o);
           var isArray = Array.isArray(o);
           for (var i = 0; i < names.length; ++i) {
             // Skip array elements indexes.
             if (isArray && /^[0-9]/.test(names[i]))
               continue;
-            resultSet[names[i]] = true;
+            group.items[group.items.length] = names[i];
           }
         }
       } catch (e) {
       }
-      return resultSet;
+      return result;
     }
 
     /**
@@ -164,6 +175,35 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
   }
 
   /**
+   * @param {!SDK.DebuggerModel.CallFrame} callFrame
+   * @param {function(!Array<!Components.JavaScriptAutocomplete.CompletionGroup>)} callback
+   */
+  function variableNamesInScopes(callFrame, callback) {
+    var result = [{items: ['this']}];

[einbinder, 2016/12/07 20:10:50]

Should this be added to the local scope? Also, it will be added again in the
keyword group down below. Maybe it should just stay there?

[pfeldman, 2016/12/08 00:29:06]

I'd want 'this' to be high in the list in the debugging case, because it makes
more sense on a breakpoint.

+
+    /**
+     * @param {string} name
+     * @param {?Array<!SDK.RemoteObjectProperty>} properties
+     */
+    function propertiesCollected(name, properties) {
+      var group = {title: name, items: []};
+      result.push(group);
+      for (var i = 0; properties && i < properties.length; ++i)
+        group.items.push(properties[i].name);
+      if (--pendingRequests === 0)
+        callback(result);
+    }
+
+    var scopeChain = callFrame.scopeChain();
+    var pendingRequests = scopeChain.length;
+    for (var i = 0; i < scopeChain.length; ++i) {
+      var scope = scopeChain[i];
+      var object = scope.object();
+      object.getAllProperties(false, propertiesCollected.bind(null, scope.typeName()));

[einbinder, 2016/12/07 20:10:51]

Nested deep in some code, you might see many Closure subtitles stacked on top of
each other.

[pfeldman, 2016/12/08 00:29:06]

Don't do deep nesting?

[einbinder, 2016/12/08 18:51:22]

Will the order of the stacks always go Local, Closure, ..., Closure, Global? It
might be different to v8, but to me there is only one closure, which has the
variables I capture in the current function.

[pfeldman, 2016/12/08 21:26:18]

You can have multiple closures, why not? The order will be Local, Closure, ...,
Closure, Global. I am not sure what you are suggesting here.

+    }
+  }
+
+  /**
    * @param {?SDK.RemoteObject} result
    * @param {!Protocol.Runtime.ExceptionDetails=} exceptionDetails
    */
@@ -176,14 +216,15 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
   }
 
   /**
-   * @param {?Object} propertyNames
+   * @param {?Object} object
    */
-  function receivedPropertyNames(propertyNames) {
+  function receivedPropertyNames(object) {
     executionContext.target().runtimeAgent().releaseObjectGroup('completion');
-    if (!propertyNames) {
+    if (!object) {
       fufill([]);
       return;
     }
+    var propertyGroups = /** @type {!Array<!Components.JavaScriptAutocomplete.CompletionGroup>} */ (object);
     var includeCommandLineAPI = (!dotNotation && !bracketNotation);
     if (includeCommandLineAPI) {
       const commandLineAPI = [
@@ -208,11 +249,10 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
         '$$',
         '$x'
       ];
-      for (var i = 0; i < commandLineAPI.length; ++i)
-        propertyNames[commandLineAPI[i]] = true;
+      propertyGroups.push({items: commandLineAPI});
     }
     fufill(Components.JavaScriptAutocomplete._completionsForQuery(
-        dotNotation, bracketNotation, expressionString, query, Object.keys(propertyNames)));
+        dotNotation, bracketNotation, expressionString, query, propertyGroups));
   }
 };
 
@@ -221,11 +261,11 @@ Components.JavaScriptAutocomplete.completionsForExpression = function(expression
    * @param {boolean} bracketNotation
    * @param {string} expressionString
    * @param {string} query
-   * @param {!Array.<string>} properties
+   * @param {!Array<!Components.JavaScriptAutocomplete.CompletionGroup>} propertyGroups
    * @return {!UI.SuggestBox.Suggestions}
    */
 Components.JavaScriptAutocomplete._completionsForQuery = function(
-    dotNotation, bracketNotation, expressionString, query, properties) {
+    dotNotation, bracketNotation, expressionString, query, propertyGroups) {
   if (bracketNotation) {
     if (query.length && query[0] === '\'')
       var quoteUsed = '\'';
@@ -239,46 +279,50 @@ Components.JavaScriptAutocomplete._completionsForQuery = function(
       'for',   'function', 'if',     'in',       'instanceof', 'new',    'return', 'switch', 'this',
       'throw', 'try',      'typeof', 'var',      'void',       'while',  'with'
     ];
-    properties = properties.concat(keywords);
+    propertyGroups.push({items: keywords});

[einbinder, 2016/12/07 20:10:51]

Should keywords get a title?

[pfeldman, 2016/12/08 00:29:06]

Sure, done.

   }
 
-  properties.sort();
+  var result = [];
+  for (var group of propertyGroups) {
+    group.items.sort();
+    var caseSensitivePrefix = [];
+    var caseInsensitivePrefix = [];
+    var caseSensitiveAnywhere = [];
+    var caseInsensitiveAnywhere = [];
 
-  var caseSensitivePrefix = [];
-  var caseInsensitivePrefix = [];
-  var caseSensitiveAnywhere = [];
-  var caseInsensitiveAnywhere = [];
-  for (var i = 0; i < properties.length; ++i) {
-    var property = properties[i];
+    for (var property of group.items) {
+      // Assume that all non-ASCII characters are letters and thus can be used as part of identifier.
+      if (dotNotation && !/^[a-zA-Z_$\u008F-\uFFFF][a-zA-Z0-9_$\u008F-\uFFFF]*$/.test(property))
+        continue;
 
-    // Assume that all non-ASCII characters are letters and thus can be used as part of identifier.
-    if (dotNotation && !/^[a-zA-Z_$\u008F-\uFFFF][a-zA-Z0-9_$\u008F-\uFFFF]*$/.test(property))
-      continue;
+      if (bracketNotation) {
+        if (!/^[0-9]+$/.test(property))
+          property = quoteUsed + property.escapeCharacters(quoteUsed + '\\') + quoteUsed;
+        property += ']';
+      }
 
-    if (bracketNotation) {
-      if (!/^[0-9]+$/.test(property))
-        property = quoteUsed + property.escapeCharacters(quoteUsed + '\\') + quoteUsed;
-      property += ']';
-    }
+      if (property.length < query.length)
+        continue;
+      if (query.length && property.toLowerCase().indexOf(query.toLowerCase()) === -1)
+        continue;
+      // Substitute actual newlines with newline characters. @see crbug.com/498421
+      var prop = property.split('\n').join('\\n');
 
-    if (property.length < query.length)
-      continue;
-    if (query.length && property.toLowerCase().indexOf(query.toLowerCase()) === -1)
-      continue;
-    // Substitute actual newlines with newline characters. @see crbug.com/498421
-    var prop = property.split('\n').join('\\n');
-
-    if (property.startsWith(query))
-      caseSensitivePrefix.push(prop);
-    else if (property.toLowerCase().startsWith(query.toLowerCase()))
-      caseInsensitivePrefix.push(prop);
-    else if (property.indexOf(query) !== -1)
-      caseSensitiveAnywhere.push(prop);
-    else
-      caseInsensitiveAnywhere.push(prop);
+      if (property.startsWith(query))
+        caseSensitivePrefix.push(prop);
+      else if (property.toLowerCase().startsWith(query.toLowerCase()))
+        caseInsensitivePrefix.push(prop);
+      else if (property.indexOf(query) !== -1)
+        caseSensitiveAnywhere.push(prop);
+      else
+        caseInsensitiveAnywhere.push(prop);
+    }
+    var structuredGroup =
+        caseSensitivePrefix.concat(caseInsensitivePrefix, caseSensitiveAnywhere, caseInsensitiveAnywhere)
+            .map(item => ({title: item}));

[einbinder, 2016/12/07 20:10:51]

Add higher priority to the case sensitive and prefix suggestions, so they are
selected by default.

caseSensitivePrefix.map(item => ({title: item, priority: 4}))
caseInsensitivePrefix.map(item => ({title: item, priority: 3}))
etc..

Otherwise document.rep will select onbeforepaste ahead of replaceChild

[pfeldman, 2016/12/08 00:29:06]

Wouldn't that break the grouping?

[einbinder, 2016/12/08 18:51:22]

No, it preserves the order of the suggestions. It just picks a different one to
be selected by default. Like we do with "width" when you type "w" in styles.

[pfeldman, 2016/12/08 21:26:18]

Done.

+    if (structuredGroup.length)
+      structuredGroup[0].subtitle = group.title;
+    result = result.concat(structuredGroup);
   }
-  return caseSensitivePrefix.concat(caseInsensitivePrefix)
-      .concat(caseSensitiveAnywhere)
-      .concat(caseInsensitiveAnywhere)
-      .map(completion => ({title: completion}));
+  return result;

[einbinder, 2016/12/07 20:10:50]

You will have duplicate suggestions if the prototype and object share a
property.

[einbinder, 2016/12/07 20:10:51]

You will have duplicate suggestions if the prototype and object share a
property.

[pfeldman, 2016/12/08 00:29:06]

Maybe that is ok?

[pfeldman, 2016/12/08 00:29:06]

Maybe that is ok?

[einbinder, 2016/12/08 18:51:22]

While it does get used for introspection, the main use of autocomplete is
autocomplete. I don't want to get suggestions for properties that I can't access
right now.

[pfeldman, 2016/12/08 21:26:18]

My worry is that with this new structure, not showing these properties could
lead to more confusion. At least when you just type "." in this edge case, you
might want to see dupes.

 };