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..aa53fb3a5d1d9b205855ffb96b4bdfe135a6a253 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
@@ -98,7 +99,7 @@ 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});

[Jacob, 2016/06/27 23:12:04]

run dartfmt

[bmilligan, 2016/06/28 01:16:35]

Done.

 
   // Define equality and hashCode so that NameValuePair can be used
   // in a Set to dedupe entries with duplicate names.
@@ -108,6 +109,9 @@ class NameValuePair {
   final String name;
   final Object value;
   final JsonMLConfig config;
+  final bool hideName;
+
+  String get displayName => hideName ? null : name;

[Jacob, 2016/06/27 23:12:04]

would '' be cleaner than null? In general prefer to avoid null values.

[bmilligan, 2016/06/28 01:16:36]

Done.

 }
 
 class MapEntry {
@@ -117,6 +121,14 @@ class MapEntry {
   final Object value;
 }
 
+class IterableSpan {
+  IterableSpan({this.low, this.high, this.object});
+
+  final int low;

[Jacob, 2016/06/27 23:12:04]

instead of low and high
use 
start, end

instead of
object
use 
iterable

[bmilligan, 2016/06/28 01:16:35]

Done.

+  final int high;
+  final Iterable object;
+}
+
 class ClassMetadata {
   ClassMetadata(this.object);
 
@@ -238,7 +250,7 @@ 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 != null ? child.displayName + ': ' : '')

[Jacob, 2016/06/27 23:12:04]

not your fault but change from 
child.displayName + ': '
to 
'${child.displayName}: '

[Jacob, 2016/06/27 23:12:04]

change to 
child.displayName.isEmpty ? '' : ...

[bmilligan, 2016/06/28 01:16:35]

Done.

[bmilligan, 2016/06/28 01:16:36]

Should it be isNotEmpty?

         ..setStyle('color: rgb(136, 19, 145);');
       if (_typeof(child.value) == 'object' ||
           _typeof(child.value) == 'function') {
@@ -279,9 +291,10 @@ class DartFormatter {
       new MapFormatter(),
       new IterableFormatter(),
       new MapEntryFormatter(),
+      new IterableSpanFormatter(),
       new ClassMetadataFormatter(),
       new HeritageClauseFormatter(),
-      new ObjectFormatter()
+      new ObjectFormatter(),
     ];
   }
 
@@ -494,19 +507,8 @@ 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(iterableChildren(new IterableSpan(low: 0, high: object.length-1, object: object)));

[Jacob, 2016/06/27 23:12:04]

no real need for method names to indicate the type of the arguments they take.
Perhaps just call this childrenHelper
I can't think of a good descriptive name. Perhaps Alan has a better idea.

[bmilligan, 2016/06/28 01:16:35]

Done.

     // TODO(jacobr): provide a link to show regular class properties here.
     // required for subclasses of iterable, etc.
     addMetadataChildren(object, ret);
@@ -607,6 +609,40 @@ class HeritageClauseFormatter implements Formatter {
   }
 }
 
+/// Formatter for synthetic MapEntry objects used to display contents of a Map

[Jacob, 2016/06/27 23:12:04]

This comment is wrong. Update it to be about your class.

[bmilligan, 2016/06/28 01:16:36]

Done.

+/// cleanly.
+class IterableSpanFormatter implements Formatter {
+  accept(object) => object is IterableSpan;
+
+  String preview(object) {
+    IterableSpan entry = object;
+    return '[${object.low}...${object.high}]';
+  }
+
+  bool hasChildren(object) => true;
+
+  List<NameValuePair> children(object) => iterableChildren(object);
+}
+
+List<NameValuePair> iterableChildren(IterableSpan span) {
+  int range = span.high - span.low + 1;

[Jacob, 2016/06/27 23:12:04]

this should be called length not range.
Also, no need for types on the lhs here and elsewhere in this method as they can
be inferred.
For consistency with other Dart apis, keep the spans exclusive rather than
inclusive. thus as a span should be from
span.start to span.end - 1
in which case
int length = span.end - span.start

[bmilligan, 2016/06/28 01:16:36]

Done.

+  List<NameValuePair> ret = new List<NameValuePair>();
+  if (range <= 100) {

[Jacob, 2016/06/27 23:12:04]

make 100 a constant

[bmilligan, 2016/06/28 01:16:35]

Done.

+    for(int i = span.low; i < span.high+1; i++) {

[Jacob, 2016/06/27 23:12:04]

should be 
i < span.end 

[bmilligan, 2016/06/28 01:16:36]

Done.

+      ret.add(new NameValuePair(name: i.toString(), value: span.object.elementAt(i)));
+    }
+  } else {
+    int a = (log(range-1)/log(100)).truncate();

[Jacob, 2016/06/27 23:12:04]

what is a? can you write this code so it is clearer?
at minimum this variable name should be easier to understand.

[bmilligan, 2016/06/28 01:16:35]

Done.

+    for (int i = span.low; i < span.high; i += pow(100, a)) {

[Jacob, 2016/06/27 23:12:04]

nit: assign pow(100, a) to a variable rather than repeatedly computing it.

[bmilligan, 2016/06/28 01:16:35]

Done.

+      int endIndex = min(span.high - i + 1, pow(100, a)) + i - 1;

[Jacob, 2016/06/27 23:12:04]

I expect by defining spans to be exclusive rather than inclusive you will
simplify these expressions. I'll review them more carefully once the logic is
updated..

[bmilligan, 2016/06/28 01:16:35]

Done.

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