Make html lists print a nice toString method.

Make html lists print a nice toString method.


BUG=

Committed: https://code.google.com/p/dart/source/detail?r=21010

[Emily Fortuna, 2013-04-05 21:50:18]

Fix for https://code.google.com/p/dart/issues/detail?id=9646 "[dart:html] .nodes
collection should implement toString"

This results in a pretty sizable code size increase however. 

An intermediary option would be to not add the toString method to the
immutable_list_mixin.dart file, so we only add the method for List<Node> and
List<Element>. 

Also, this change is not tremendously helpful until we also add some toString
methods for various Elements as well, but that's for another CL.

[Jennifer Messerly, 2013-04-05 22:02:31]

On 2013/04/05 21:50:18, Emily Fortuna wrote:
> Fix for https://code.google.com/p/dart/issues/detail?id=9646 "[dart:html]
.nodes
> collection should implement toString"
> 
> This results in a pretty sizable code size increase however. 
> 
> An intermediary option would be to not add the toString method to the
> immutable_list_mixin.dart file, so we only add the method for List<Node> and
> List<Element>. 
> 
> Also, this change is not tremendously helpful until we also add some toString
> methods for various Elements as well, but that's for another CL.

Doh. Forgot about the code size problem! Maybe not such a good idea? :|

fwiw, a nice Element.toString() would be outerHtml. That's what the Chrome
debugger console does for HTML elements.

[Jennifer Messerly, 2013-04-05 22:03:57]

On 2013/04/05 22:02:31, John Messerly wrote:
> On 2013/04/05 21:50:18, Emily Fortuna wrote:
> > Fix for https://code.google.com/p/dart/issues/detail?id=9646 "[dart:html]
> .nodes
> > collection should implement toString"
> > 
> > This results in a pretty sizable code size increase however. 
> > 
> > An intermediary option would be to not add the toString method to the
> > immutable_list_mixin.dart file, so we only add the method for List<Node> and
> > List<Element>. 
> > 
> > Also, this change is not tremendously helpful until we also add some
toString
> > methods for various Elements as well, but that's for another CL.
> 
> Doh. Forgot about the code size problem! Maybe not such a good idea? :|
> 
> fwiw, a nice Element.toString() would be outerHtml. That's what the Chrome
> debugger console does for HTML elements.

Also, what is "sizable increase"?
If you use toString on something else (say, ints) is dart2js smart enough to
figure out that it doesn't need to compile the DOM one? Or does .toString always
cause the code to be included?

[blois, 2013-04-05 22:05:27]

Will toString be part of the standard List mixin?

If so, then this will go away. If not, then I think we'd just need to add our
own mixin for it, though I wonder why it would not be in the standard one.

I'm quite a bit more worried about adding toString for all elements.

[blois, 2013-04-05 22:08:15]

lgtm

Dart2js is smart enough in some cases, but the further you get away from where
the type is declared more info gets lost and dart2js may not know which toString
is actually being used.

With mixins, the overhead of this should be negligible though.

[Jennifer Messerly, 2013-04-05 22:12:47]

On 2013/04/05 22:08:15, blois wrote:
> lgtm
> 
> Dart2js is smart enough in some cases, but the further you get away from where
> the type is declared more info gets lost and dart2js may not know which
toString
> is actually being used.
> 
> With mixins, the overhead of this should be negligible though.

sounds good to me. thanks!

[Jennifer Messerly, 2013-04-05 22:13:22]

On 2013/04/05 22:12:47, John Messerly wrote:
> On 2013/04/05 22:08:15, blois wrote:
> > lgtm
> > 
> > Dart2js is smart enough in some cases, but the further you get away from
where
> > the type is declared more info gets lost and dart2js may not know which
> toString
> > is actually being used.
> > 
> > With mixins, the overhead of this should be negligible though.
> 
> sounds good to me. thanks!

yeah, I was mainly hoping that printing lists would be more helpful, I was
debugging and had to do .toList() which felt strange :)

[Emily Fortuna, 2013-04-05 22:18:32]

On 2013/04/05 22:05:27, blois wrote:
> Will toString be part of the standard List mixin?
> 
> If so, then this will go away. If not, then I think we'd just need to add our
> own mixin for it, though I wonder why it would not be in the standard one.
> 
> I'm quite a bit more worried about adding toString for all elements.

Pete:
So here's the deal: 
There's an "IterableMixin" that does not have a nicer toString method as part of
its interface. Collection is the one that has the nice toString method. However,
the inheritance hierarchy goes like this: Iterable>Collection>List. So List
should get the nicer toString implmentation since it is a Collection... but  I
don't see any plain-jane List or Collection mixin itself (aside from the one
we've created in dart_html). 

Re: John -- about the code size increase. The reason we have this code size
increase is the "Lists" in dart:html don't actually extend from List. They
implement list, and act like normal lists but aren't really (because the
underlying DOM List types are not always quite like Dart Lists).  We implement
this in dart:html by adding the List methods to every List-like object in
dart:html, mixin style, but as you see in html_dart2js.dart and
html_dartium.dart this change added 244 more lines in each file. So, on the
upside, ints we wouldn't need to worry about for code size increase. It's just
the Lists and Element/Node.

[Emily Fortuna, 2013-04-05 23:53:47]

Committed patchset #2 manually as r21010 (presubmit successful).

[sra1, 2013-04-08 17:00:47]

I'm seeing a size impact in swarm: +3k

This could be mitigated by having all these new toStrings call the common
collections version that is used by other collections, 

  String toString() => Collections.collectionToString(this);