Refactor command code and add support for --help and --version.

Refactor command code and add support for --help and --version.

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

[Bob Nystrom, 2012-04-25 00:38:18]

This includes the changes from the previous pub update. You can ignore those.
I'll rebase once that patch goes in.

[nweiz, 2012-04-25 20:49:41]

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
utils/pub/command_list.dart:7: String get description() => 'print the contents
of repositories';
This isn't really accurate... currently what this does is prints the contents of
the cache.

I'm not sure that's actually the most useful behavior, though. The cache should
be largely transparent to the user. It may make sense as an option, but it seems
like a more useful default would be to print the transitive dependency list of
the current package.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_upda...
File utils/pub/command_update.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_upda...
utils/pub/command_update.dart:7: String get description() => "update a package's
dependencies";
"update the current package's dependencies to the latest versions"?

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/pub.dart#new...
utils/pub/pub.dart:38: if (args.length == 1 && args[0] == '--help') {
Also support -h

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
File utils/tests/pub/pub_tests.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
utils/tests/pub/pub_tests.dart:23: version   print Pub version
Since this output isn't sorted, this test is potentially flaky.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
utils/tests/pub/pub_tests.dart:39: output: USAGE_STRING);
May as well test for just "help" too.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/test_p...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:199: abstract Future<bool> validate(String path);
I'd make it clearer that path is always a directory containing this
file/directory. Maybe call it "dir" and say "...matches the corresponding file
system entry within [dir]."

[Bob Nystrom, 2012-04-25 22:56:40]

Thanks!

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
utils/pub/command_list.dart:7: String get description() => 'print the contents
of repositories';
On 2012/04/25 20:49:41, nweiz wrote:
> This isn't really accurate... currently what this does is prints the contents
of
> the cache.

Right.

> 
> I'm not sure that's actually the most useful behavior, though. The cache
should
> be largely transparent to the user. It may make sense as an option, but it
seems
> like a more useful default would be to print the transitive dependency list of
> the current package.

Yeah, this command basically exists so that we can get some information out of
pub for some of the early integration tests. If you'd like, we can give it a
more obvious temp name like "debuglist". It's basically here so that I could
write tests of the basic package loading code before any other useful commands
had been written.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_upda...
File utils/pub/command_update.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_upda...
utils/pub/command_update.dart:7: String get description() => "update a package's
dependencies";
On 2012/04/25 20:49:41, nweiz wrote:
> "update the current package's dependencies to the latest versions"?

Done.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/pub.dart#new...
utils/pub/pub.dart:38: if (args.length == 1 && args[0] == '--help') {
On 2012/04/25 20:49:41, nweiz wrote:
> Also support -h

Done.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
File utils/tests/pub/pub_tests.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
utils/tests/pub/pub_tests.dart:23: version   print Pub version
On 2012/04/25 20:49:41, nweiz wrote:
> Since this output isn't sorted, this test is potentially flaky.

It is sorted. See pub.dart:98.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
utils/tests/pub/pub_tests.dart:39: output: USAGE_STRING);
On 2012/04/25 20:49:41, nweiz wrote:
> May as well test for just "help" too.

Help will be a full command (so you can do "help [command]" too, and it hasn't
been implemented yet. :)

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/test_p...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:199: abstract Future<bool> validate(String path);
On 2012/04/25 20:49:41, nweiz wrote:
> I'd make it clearer that path is always a directory containing this
> file/directory. Maybe call it "dir" and say "...matches the corresponding file
> system entry within [dir]."

Done.

[nweiz, 2012-04-25 23:09:22]

lgtm

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
utils/pub/command_list.dart:7: String get description() => 'print the contents
of repositories';
On 2012/04/25 22:56:40, Bob Nystrom wrote:
> On 2012/04/25 20:49:41, nweiz wrote:
> > This isn't really accurate... currently what this does is prints the
contents
> of
> > the cache.
> 
> Right.
> 
> > 
> > I'm not sure that's actually the most useful behavior, though. The cache
> should
> > be largely transparent to the user. It may make sense as an option, but it
> seems
> > like a more useful default would be to print the transitive dependency list
of
> > the current package.
> 
> Yeah, this command basically exists so that we can get some information out of
> pub for some of the early integration tests. If you'd like, we can give it a
> more obvious temp name like "debuglist". It's basically here so that I could
> write tests of the basic package loading code before any other useful commands
> had been written.

Oh, sure. Maybe just add a comment about that here.

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
File utils/tests/pub/pub_tests.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/tests/pub/pub_te...
utils/tests/pub/pub_tests.dart:23: version   print Pub version
On 2012/04/25 22:56:40, Bob Nystrom wrote:
> On 2012/04/25 20:49:41, nweiz wrote:
> > Since this output isn't sorted, this test is potentially flaky.
> 
> It is sorted. See pub.dart:98.

Oh, I see. I was confused by the TODO about the sorted map.

[Bob Nystrom, 2012-04-26 00:22:13]

Thanks!

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
File utils/pub/command_list.dart (right):

https://chromiumcodereview.appspot.com/10214006/diff/1/utils/pub/command_list...
utils/pub/command_list.dart:7: String get description() => 'print the contents
of repositories';
On 2012/04/25 23:09:22, nweiz wrote:
> On 2012/04/25 22:56:40, Bob Nystrom wrote:
> > On 2012/04/25 20:49:41, nweiz wrote:
> > > This isn't really accurate... currently what this does is prints the
> contents
> > of
> > > the cache.
> > 
> > Right.
> > 
> > > 
> > > I'm not sure that's actually the most useful behavior, though. The cache
> > should
> > > be largely transparent to the user. It may make sense as an option, but it
> > seems
> > > like a more useful default would be to print the transitive dependency
list
> of
> > > the current package.
> > 
> > Yeah, this command basically exists so that we can get some information out
of
> > pub for some of the early integration tests. If you'd like, we can give it a
> > more obvious temp name like "debuglist". It's basically here so that I could
> > write tests of the basic package loading code before any other useful
commands
> > had been written.
> 
> Oh, sure. Maybe just add a comment about that here.

Done.