pkg/args: expose immutable collections

pkg/args: expose immutable collections

BUG= https://code.google.com/p/dart/issues/detail?id=10127
R=rnystrom@google.com

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

[kevmoo-old, 2013-07-23 03:06:41]

PTAL

[kevmoo-old, 2013-07-23 14:05:36]

I have a few test failures w/ Pub. Seems it's using 'rest' as mutable.
Investigating.

[kevmoo-old, 2013-07-23 14:23:09]

Fixed.

[Bob Nystrom, 2013-07-23 15:36:08]

Thanks for doing this!

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart
File pkg/args/lib/args.dart (right):

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:270: 
Nit: remove this blank line.

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:285: factory ArgParser() => new ArgParser._(<String,
Option>{}, <String, ArgParser>{});
Long line. Wrap it after "=>" (and indent +4).

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:291: this.commands = new UnmodifiableMapView(commands);
Ugh, constructor initialization lists for the lose. This is the best way to do
this, but what a pain.

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:432: ArgResults(this._options, this.name, this.command,
List<String> rest) :
Style nit: move the ":" to the next line, like:

ArgResults(...)
    : this.rest = ...

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/src/parser.dart
File pkg/args/lib/src/parser.dart (right):

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/src/parser.da...
pkg/args/lib/src/parser.dart:280: void validate(bool condition, String message)
{
Good catch.

https://codereview.chromium.org/19810004/diff/9001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/command/uploader.dart (right):

https://codereview.chromium.org/19810004/diff/9001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/command/uploader.dart:47: var rest = new
List<String>.from(commandOptions.rest);
You can do this instead:

var rest = commandOptions.rest.toList();

[kevmoo-old, 2013-07-23 15:49:13]

PTAL

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart
File pkg/args/lib/args.dart (right):

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:270: 
On 2013/07/23 15:36:08, Bob Nystrom wrote:
> Nit: remove this blank line.

Done.

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:285: factory ArgParser() => new ArgParser._(<String,
Option>{}, <String, ArgParser>{});
On 2013/07/23 15:36:08, Bob Nystrom wrote:
> Long line. Wrap it after "=>" (and indent +4).

Done.

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:291: this.commands = new UnmodifiableMapView(commands);
On 2013/07/23 15:36:08, Bob Nystrom wrote:
> Ugh, constructor initialization lists for the lose. This is the best way to do
> this, but what a pain.

Agreed.

https://codereview.chromium.org/19810004/diff/9001/pkg/args/lib/args.dart#new...
pkg/args/lib/args.dart:432: ArgResults(this._options, this.name, this.command,
List<String> rest) :
On 2013/07/23 15:36:08, Bob Nystrom wrote:
> Style nit: move the ":" to the next line, like:
> 
> ArgResults(...)
>     : this.rest = ...

Done.

https://codereview.chromium.org/19810004/diff/9001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/command/uploader.dart (right):

https://codereview.chromium.org/19810004/diff/9001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/command/uploader.dart:47: var rest = new
List<String>.from(commandOptions.rest);
On 2013/07/23 15:36:08, Bob Nystrom wrote:
> You can do this instead:
> 
> var rest = commandOptions.rest.toList();

Done.

[Bob Nystrom, 2013-07-23 16:15:04]

LGTM!

[kevmoo-old, 2013-07-23 16:56:17]

Committed patchset #6 manually as r25347 (presubmit successful).