Fixed YAML output for a list of maps

Fixed YAML output for a list of maps

R=alanknight@google.com, efortuna@google.com

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

[Tate Mandel, 2013-07-24 23:24:36]

This updates the dart2yaml library to output valid YAML for a list of maps by
adding proper spacing and dashes.

[Alan Knight, 2013-07-25 17:46:42]

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart
File pkg/docgen/lib/dart2yaml.dart (right):

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart...
pkg/docgen/lib/dart2yaml.dart:22: */
This could use some documentation for the parameters. With "isList" it isn't
obvious what's a list.

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart...
pkg/docgen/lib/dart2yaml.dart:27: if (isList && key == documentData.keys.first)
{
Since the order of keys is non-deterministic, I assume this means documentData
should always have one element?

In general, we seem to have some strong assumptions about the structure of
documentData, and those should probably be documented.

[Tate Mandel, 2013-07-25 22:01:53]

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart
File pkg/docgen/lib/dart2yaml.dart (right):

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart...
pkg/docgen/lib/dart2yaml.dart:22: */
On 2013/07/25 17:46:42, Alan Knight wrote:
> This could use some documentation for the parameters. With "isList" it isn't
> obvious what's a list.

Done.

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart...
pkg/docgen/lib/dart2yaml.dart:27: if (isList && key == documentData.keys.first)
{
On 2013/07/25 17:46:42, Alan Knight wrote:
> Since the order of keys is non-deterministic, I assume this means documentData
> should always have one element?
> 
> In general, we seem to have some strong assumptions about the structure of
> documentData, and those should probably be documented.

documentData can have many elements, but the first one should be marked with a
'-' to denote that it's the start of a new record in the collection. See example
2.4 at http://www.yaml.org/spec/1.2/spec.html#id2759963. 

If we are outputting YAML for a list of maps, we have to mark the first key of
each map with a '-' to denote that it's the start of a separate member of that
list.

If it is non-deterministic, then maybe we should alphabetize keys first to
ensure the same key is at the beginning for each member of the outer list?

[Emily Fortuna, 2013-07-25 23:20:09]

lgtm

[Alan Knight, 2013-07-25 23:25:04]

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart
File pkg/docgen/lib/dart2yaml.dart (right):

https://codereview.chromium.org/20162006/diff/1/pkg/docgen/lib/dart2yaml.dart...
pkg/docgen/lib/dart2yaml.dart:27: if (isList && key == documentData.keys.first)
{
On 2013/07/25 22:01:53, Tate Mandel wrote:
> On 2013/07/25 17:46:42, Alan Knight wrote:
> > Since the order of keys is non-deterministic, I assume this means
documentData
> > should always have one element?
> > 
> > In general, we seem to have some strong assumptions about the structure of
> > documentData, and those should probably be documented.
> 
> documentData can have many elements, but the first one should be marked with a
> '-' to denote that it's the start of a new record in the collection. See
example
> 2.4 at http://www.yaml.org/spec/1.2/spec.html#id2759963. 
> 
> If we are outputting YAML for a list of maps, we have to mark the first key of
> each map with a '-' to denote that it's the start of a separate member of that
> list.
> 
> If it is non-deterministic, then maybe we should alphabetize keys first to
> ensure the same key is at the beginning for each member of the outer list?

I think the ordering is actually quite messy. If I understand, the ordering for
a literal map is deterministic, because it is in Javascript and that was likely
to break people's assumptions. And there's obvious a class that preserves that
ordering, even though what's underneath is a hash table, and doesn't. So if
we're relying on the "first" key in a map, we should make sure we're always
going to be using the kind of map that preserves that order, or else doing
something like sorting ourselves to ensure it.

[Tate Mandel, 2013-07-26 20:33:04]

https://chromiumcodereview.appspot.com/20162006/diff/1/pkg/docgen/lib/dart2ya...
File pkg/docgen/lib/dart2yaml.dart (right):

https://chromiumcodereview.appspot.com/20162006/diff/1/pkg/docgen/lib/dart2ya...
pkg/docgen/lib/dart2yaml.dart:27: if (isList && key == documentData.keys.first)
{
On 2013/07/25 23:25:04, Alan Knight wrote:
> On 2013/07/25 22:01:53, Tate Mandel wrote:
> > On 2013/07/25 17:46:42, Alan Knight wrote:
> > > Since the order of keys is non-deterministic, I assume this means
> documentData
> > > should always have one element?
> > > 
> > > In general, we seem to have some strong assumptions about the structure of
> > > documentData, and those should probably be documented.
> > 
> > documentData can have many elements, but the first one should be marked with
a
> > '-' to denote that it's the start of a new record in the collection. See
> example
> > 2.4 at http://www.yaml.org/spec/1.2/spec.html#id2759963. 
> > 
> > If we are outputting YAML for a list of maps, we have to mark the first key
of
> > each map with a '-' to denote that it's the start of a separate member of
that
> > list.
> > 
> > If it is non-deterministic, then maybe we should alphabetize keys first to
> > ensure the same key is at the beginning for each member of the outer list?
> 
> I think the ordering is actually quite messy. If I understand, the ordering
for
> a literal map is deterministic, because it is in Javascript and that was
likely
> to break people's assumptions. And there's obvious a class that preserves that
> ordering, even though what's underneath is a hash table, and doesn't. So if
> we're relying on the "first" key in a map, we should make sure we're always
> going to be using the kind of map that preserves that order, or else doing
> something like sorting ourselves to ensure it.

Done.

[Alan Knight, 2013-07-26 20:51:03]

lgtm

https://chromiumcodereview.appspot.com/20162006/diff/11001/pkg/docgen/lib/dar...
File pkg/docgen/lib/dart2yaml.dart (right):

https://chromiumcodereview.appspot.com/20162006/diff/11001/pkg/docgen/lib/dar...
pkg/docgen/lib/dart2yaml.dart:32: keys.sort((String first, String second) =>
first.compareTo(second));
Real nit. You can omit the argument to sort and the default will work for
sorting strings in alphabetical order.

[Tate Mandel, 2013-07-26 21:00:32]

Committed patchset #3 manually as r25539 (presubmit successful).