search

dart-lang / yaml

A Dart YAML parser.
https://pub.dev/packages/yaml
MIT License
169 stars 58 forks source link

Change String or Uri arguments to Uri #97

Closed natebosch closed 3 years ago

natebosch commented 3 years ago

Adding a static type makes the API easier to use correctly. The call site can easily inline the Uri.parse if necessary.

kevmoo commented 3 years ago

Why not Object? – this is going to be annoying to deal with.

We didn't make the same update to SpanScanner.eager in StringScanner

kevmoo commented 3 years ago

I wish we had https://github.com/dart-lang/language/blob/master/working/0107%20-%20implicit-constructors/feature-brainstorm.md#syntax

natebosch commented 3 years ago

Why not Object?

Because that doesn't add any safety from the calling side. It doesn't stop me from accidentally passing in a File or something weird.

this is going to be annoying to deal with.

This is the direction we are moving with all arguments that unnecessarily are dynamic or Object to allow either String or Uri. Why do you think it will be annoying?

In all internal uses of this package, there was only a single library that I found was impacted. I did not scan on pub, but I don't expect there to be much impact.

The fix is also straightforward. https://dart-review.googlesource.com/c/sdk/+/171440

We didn't make the same update to SpanScanner.eager in StringScanner

We will. https://github.com/dart-lang/string_scanner/issues/30

kevmoo commented 3 years ago

ack...alright.