Array formatting customized to look like JS

tool/input_sdk/private/debugger.dart

Index: tool/input_sdk/private/debugger.dart
diff --git a/tool/input_sdk/private/debugger.dart b/tool/input_sdk/private/debugger.dart
index e7fb00aea1709f82efdad69c2485c01c65662ef4..94542656cd2b41ae34df8695834b463f46abc3ab 100644
--- a/tool/input_sdk/private/debugger.dart
+++ b/tool/input_sdk/private/debugger.dart
@@ -9,6 +9,7 @@ import 'dart:_runtime' as dart;
 import 'dart:core';
 import 'dart:collection';
 import 'dart:html' as html;
+import 'dart:math';
 
 /// JsonMLConfig object to pass to devtools to specify how an Object should
 /// be displayed. skipDart signals that an object should not be formatted
@@ -29,7 +30,7 @@ class JsonMLConfig {
   static const keyToString = const JsonMLConfig("keyToString");
 }
 
-final int maxIterableChildrenToDisplay = 50;
+int maxIterableChildrenSubset = 100;
 
 var _devtoolsFormatter = new JsonMLFormatter(new DartFormatter());
 
@@ -98,7 +99,11 @@ bool hasMethod(object, String name) {
 
 /// [JsonMLFormatter] consumes [NameValuePair] objects and
 class NameValuePair {
-  NameValuePair({this.name, this.value, this.config: JsonMLConfig.none});
+  NameValuePair(
+      {this.name,
+      this.value,
+      this.config: JsonMLConfig.none,
+      this.hideName: false});
 
   // Define equality and hashCode so that NameValuePair can be used
   // in a Set to dedupe entries with duplicate names.
@@ -108,6 +113,9 @@ class NameValuePair {
   final String name;
   final Object value;
   final JsonMLConfig config;
+  final bool hideName;
+
+  String get displayName => hideName ? '' : name;
 }
 
 class MapEntry {
@@ -117,6 +125,14 @@ class MapEntry {
   final Object value;
 }
 
+class IterableSpan {
+  IterableSpan({this.start, this.end, this.iterable});
+
+  final int start;
+  final int end;
+  final Iterable iterable;
+}
+
 class ClassMetadata {
   ClassMetadata(this.object);
 
@@ -238,7 +254,8 @@ class JsonMLFormatter {
     for (NameValuePair child in children) {
       var li = body.createChild('li');
       var nameSpan = new JsonMLElement('span')
-        ..createTextChild(child.name != null ? child.name + ': ' : '')
+        ..createTextChild(
+            child.displayName.isNotEmpty ? '${child.displayName}: ' : '')
         ..setStyle('color: rgb(136, 19, 145);');
       if (_typeof(child.value) == 'object' ||
           _typeof(child.value) == 'function') {
@@ -279,9 +296,10 @@ class DartFormatter {
       new MapFormatter(),
       new IterableFormatter(),
       new MapEntryFormatter(),
+      new IterableSpanFormatter(),
       new ClassMetadataFormatter(),
       new HeritageClauseFormatter(),
-      new ObjectFormatter()
+      new ObjectFormatter(),
     ];
   }
 
@@ -494,19 +512,9 @@ class IterableFormatter extends ObjectFormatter {
     // are not the built in Set or List types.
     // TODO(jacobr): handle large Iterables better.
     // TODO(jacobr): consider only using numeric indices
-    Iterable iterable = object;
     var ret = new LinkedHashSet<NameValuePair>();
-    var i = 0;
-    for (var entry in iterable) {
-      if (i > maxIterableChildrenToDisplay) {
-        ret.add(new NameValuePair(
-            name: 'Warning', value: 'Truncated Iterable display'));
-        // TODO(jacobr): provide an expandable entry to show more entries.
-        break;
-      }
-      ret.add(new NameValuePair(name: i.toString(), value: entry));
-      i++;
-    }
+    ret.addAll(childrenHelper(
+        new IterableSpan(start: 0, end: object.length, iterable: object)));
     // TODO(jacobr): provide a link to show regular class properties here.
     // required for subclasses of iterable, etc.
     addMetadataChildren(object, ret);
@@ -607,6 +615,46 @@ class HeritageClauseFormatter implements Formatter {
   }
 }
 
+/// Formatter for synthetic IterableSpan objects used to display contents of
+/// an Iterable cleanly.
+class IterableSpanFormatter implements Formatter {
+  accept(object) => object is IterableSpan;
+
+  String preview(object) {
+    IterableSpan entry = object;
+    return '[${object.start}...${object.end-1}]';
+  }
+
+  bool hasChildren(object) => true;
+
+  List<NameValuePair> children(object) => childrenHelper(object);
+}
+
+List<NameValuePair> childrenHelper(IterableSpan span) {
+  var length = span.end - span.start;
+  var ret = new List<NameValuePair>();
+  if (length <= maxIterableChildrenSubset) {

[Jacob, 2016/06/28 16:35:54]

how about 
maxBlockLength
or
maxSpanLength
instead
of maxIterableChildrenSubset?

[bmilligan, 2016/06/28 17:26:09]

Done.

+    for (int i = span.start; i < span.end; i++) {

[Jacob, 2016/06/28 16:35:54]

var i
instead of int i

[bmilligan, 2016/06/28 17:26:09]

Done.

+      ret.add(new NameValuePair(
+          name: i.toString(), value: span.iterable.elementAt(i)));

[Jacob, 2016/06/28 16:35:54]

Interesting algorithmic complexity case to consider: if an iterable happens to
be a LinkedList or other iterable where elementAt is O(n) this could be quite
slow.
You could test how noticeable the slowdown is for a linked list with 10M
elements. If the UI is still usable then don't worry about it. If the UI hangs,
use iterable.skip and 
for (var element in iterable.skip(start)) {

[bmilligan, 2016/06/28 17:26:09]

I created a ListQueue of ~40M nodes and there was no delay in the UI for the
custom formatters. I also tested using iterable.skip and that created lag in the
UI, so I suppose I should stick with this algorithm.

[Jacob, 2016/06/28 17:37:02]

Go ahead and just add a TODO to stop using elementAt if it becomes a performance
bottlekneck in the future.

[bmilligan, 2016/06/28 17:55:06]

Done.

+    }
+  } else {
+    var maxPowerOfSubsetSize =
+        (log(length - 1) / log(maxIterableChildrenSubset)).truncate();

[Jacob, 2016/06/28 16:35:54]

why is this log(length - 1) instead of log(length)?
length - 1 may be right, it should be documented.
I imagine it is length - 1 so that 100 gets 1 still returns 0 instead of 1?

[bmilligan, 2016/06/28 17:26:09]

Yes, the -1 is dealing with the perfect square case so that a list of length
10000 results in a maxPowerOfSubsetSize of 1 instead of 2, and therefore will be
broken up into subsets of 100 instead of creating a loop of subsets of 10000
that never get broken up.

+    var subsize = pow(maxIterableChildrenSubset, maxPowerOfSubsetSize);

[Jacob, 2016/06/28 16:35:54]

camel case and avoid abbreviations for variable names. I recommend reading
through https://www.dartlang.org/effective-dart/style/

[bmilligan, 2016/06/28 17:26:09]

Done.

+    for (int i = span.start; i < span.end; i += subsize) {

[Jacob, 2016/06/28 16:35:54]

here and elsewhere, write 
var
instead of int 
when types will be easily inferred.

[bmilligan, 2016/06/28 17:26:09]

Done.

+      var endIndex = min(span.end - i, subsize) + i;

[Jacob, 2016/06/28 16:35:54]

I think
min(span.end, i + subsize)
is clearer

[bmilligan, 2016/06/28 17:26:09]

Done.

+      var entryWrapper =
+          new IterableSpan(start: i, end: endIndex, iterable: span.iterable);
+      ret.add(new NameValuePair(
+          name: '[${i}...${endIndex - 1}]',
+          value: entryWrapper,
+          hideName: true));
+    }
+  }
+  return ret;
+}
+
 /// This entry point is automatically invoked by the code generated by
 /// Dart Dev Compiler
 registerDevtoolsFormatter() {