Array formatting customized to look like JS

tool/input_sdk/private/debugger.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library dart._debugger;
 
 import 'dart:_foreign_helper' show JS;
 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
 /// by the Dart formatter. This is used to specify that an Object
 /// should just be displayed using the regular JavaScript view instead of a
 /// custom Dart view. For example, this is used to display the JavaScript view
 /// of a Dart Function as a child of the regular Function object. keyToString
 /// signals that a map key object should have its toString() displayed by
 /// the Dart formatter.
 ///
@@ skipping 69 matching lines @@
 bool hasMethod(object, String name) {
   try {
     return dart.hasMethod(object, name);
   } catch (e) {
     return false;
   }
 }
 
 /// [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.
   operator ==(other) => other is NameValuePair && other.name == name;
   int get hashCode => name.hashCode;
 
   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 {
   MapEntry({this.key, this.value});
 
   final Object key;
   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);
 
   final Object object;
 }
 
 class HeritageClause {
   HeritageClause(this.name, this.types);
 
   final String name;
@@ skipping 101 matching lines @@
     var body = new JsonMLElement('ol')
       ..setStyle('list-style-type: none;'
           'padding-left: 0px;'
           'margin-top: 0px;'
           'margin-bottom: 0px;'
           'margin-left: 12px');
     var children = _simpleFormatter.children(object);
     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') {
         nameSpan.addStyle("padding-left: 13px;");
 
         li.appendChild(nameSpan);
         var objectTag = li.createObjectTag(child.value);
         objectTag.addAttribute('config', child.config);
         if (!_simpleFormatter.hasChildren(child.value)) {
           li.setStyle("padding-left: 13px;");
@@ skipping 20 matching lines @@
   List<Formatter> _formatters;
 
   DartFormatter() {
     // The order of formatters matters as formatters later in the list take
     // precidence.
     _formatters = [
       new FunctionFormatter(),
       new MapFormatter(),
       new IterableFormatter(),
       new MapEntryFormatter(),
+      new IterableSpanFormatter(),
       new ClassMetadataFormatter(),
       new HeritageClauseFormatter(),
-      new ObjectFormatter()
+      new ObjectFormatter(),
     ];
   }
 
   String preview(object) {
     try {
       if (object == null ||
           object is num ||
           object is String ||
           isNativeJavaScriptObject(object)) {
         return object.toString();
@@ skipping 192 matching lines @@
     }
   }
 
   bool hasChildren(object) => true;
 
   List<NameValuePair> children(object) {
     // TODO(jacobr): be lazier about enumerating contents of Iterables that
     // 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);
     return ret.toList();
   }
 }
 
 // This class does double duting displaying metadata for
 class ClassMetadataFormatter implements Formatter {
   accept(object) => object is ClassMetadata;
@@ skipping 80 matching lines @@
   List<NameValuePair> children(object) {
     HeritageClause clause = object;
     var ret = <NameValuePair>[];
     for (var type in clause.types) {
       ret.add(new NameValuePair(value: new ClassMetadata(type)));
     }
     return ret;
   }
 }
 
+/// 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() {
   var formatters = [_devtoolsFormatter];
   JS('', 'dart.global.devtoolsFormatters = #', formatters);
 }