Attach source range information to parsed YAML nodes.

Attach source range information to parsed YAML nodes.

Also release yaml 1.0.0.

R=efortuna@google.com, rnystrom@google.com, sigmund@google.com

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

[Bob Nystrom, 2014-06-03 00:33:14]

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/CHANGELOG.md
File pkg/yaml/CHANGELOG.md (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/CHANGELOG.md#newcode4
pkg/yaml/CHANGELOG.md:4: `loadYamlStream` are now read-only.
"read-only" -> "immutable".

(Assuming their internal state doesn't change at all.)

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/composer.dart
File pkg/yaml/lib/src/composer.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/composer.da...
pkg/yaml/lib/src/composer.dart:50: return setAnchor(scalar,
parseString(scalar.content, scalar.span));
Would it make more sense to just pass the whole ScalarNode to these methods?

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
File pkg/yaml/lib/src/constructor.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:41: /// Converts a mapping into a Map of Dart
objects.
"Map" -> "[Map]".

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:51: /// Returns the Dart object that already
represents [anchored], if such a
This is a bit confusing since it's a new object now.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:57: var value = _anchors[anchored.anchor];
Can this ever be null now? If not, use it for the above early exit check.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:57: var value = _anchors[anchored.anchor];
Add a comment explaining why these are copied. Is it just so the result has the
right location?

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.dart
File pkg/yaml/lib/src/equality.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.da...
pkg/yaml/lib/src/equality.dart:100: /// [YamlScalar]s and their values.
This has scary perf implications. Can we cache this inside YamlList and YamlMap?

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.da...
pkg/yaml/lib/src/equality.dart:117: } else {
Nit, may need to handle NaN here too. :-/

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/yaml_node.dart
File pkg/yaml/lib/src/yaml_node.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/yaml_node.d...
pkg/yaml/lib/src/yaml_node.dart:75: operator []=(int index, value) {
Do you need to do the same thing with YamlMap? What about the other mutating
methods?

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback.dart:143: configuration = new
Map.from(configuration);
Document this.

Also, a nit, but can you add a blank line after this? I tend to prefer comment
lines have a blank line above them.

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/pubspec.dart (right):

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/pubspec.dart:360: spec = new Map.from(spec);
Document this.

[nweiz, 2014-06-03 02:03:21]

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/CHANGELOG.md
File pkg/yaml/CHANGELOG.md (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/CHANGELOG.md#newcode4
pkg/yaml/CHANGELOG.md:4: `loadYamlStream` are now read-only.
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> "read-only" -> "immutable".
> 
> (Assuming their internal state doesn't change at all.)

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/composer.dart
File pkg/yaml/lib/src/composer.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/composer.da...
pkg/yaml/lib/src/composer.dart:50: return setAnchor(scalar,
parseString(scalar.content, scalar.span));
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Would it make more sense to just pass the whole ScalarNode to these methods?

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
File pkg/yaml/lib/src/constructor.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:41: /// Converts a mapping into a Map of Dart
objects.
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> "Map" -> "[Map]".

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:51: /// Returns the Dart object that already
represents [anchored], if such a
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> This is a bit confusing since it's a new object now.

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:57: var value = _anchors[anchored.anchor];
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Can this ever be null now? If not, use it for the above early exit check.

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/constructor...
pkg/yaml/lib/src/constructor.dart:57: var value = _anchors[anchored.anchor];
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Add a comment explaining why these are copied. Is it just so the result has
the
> right location?

Done.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.dart
File pkg/yaml/lib/src/equality.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.da...
pkg/yaml/lib/src/equality.dart:100: /// [YamlScalar]s and their values.
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> This has scary perf implications. Can we cache this inside YamlList and
YamlMap?

Not really; they don't override [hashCode], a custom version is defined for the
inner map.

I'm not too worried about people putting really complex objects in map keys, but
one possibility would be to have the hash code just hash the number of child
objects, and rely on equality to disambiguate between similarly-sized objects.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/equality.da...
pkg/yaml/lib/src/equality.dart:117: } else {
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Nit, may need to handle NaN here too. :-/

NaN always has the same hash code, even though it's not equal to itself.

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/yaml_node.dart
File pkg/yaml/lib/src/yaml_node.dart (right):

https://codereview.chromium.org/302313007/diff/1/pkg/yaml/lib/src/yaml_node.d...
pkg/yaml/lib/src/yaml_node.dart:75: operator []=(int index, value) {
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Do you need to do the same thing with YamlMap? What about the other mutating
> methods?

All the mutating methods of List are overridden here. Map uses
UnmodifiableMapMixin to do so automatically.

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback.dart:143: configuration = new
Map.from(configuration);
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Document this.
> 
> Also, a nit, but can you add a blank line after this? I tend to prefer comment
> lines have a blank line above them.

Done.

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/pubspec.dart (right):

https://codereview.chromium.org/302313007/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/pubspec.dart:360: spec = new Map.from(spec);
On 2014/06/03 00:33:14, Bob Nystrom wrote:
> Document this.

Done.

[Bob Nystrom, 2014-06-03 20:02:41]

lgtm

[Emily Fortuna, 2014-06-03 20:28:35]

lgtm

[Siggi Cherem (dart-lang), 2014-06-03 20:30:52]

lgtm

[nweiz, 2014-06-03 20:33:46]

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