Take Sam Elkhateeb's path for "path" dependencies and clean it up some.

Take Sam Elkhateeb's path for "path" dependencies and clean it up some.

BUG=http://code.google.com/p/dart/issues/detail?id=3732

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

[Bob Nystrom, 2013-02-12 22:11:16]

I haven't added support for relative paths yet. I'll do that in a separate patch
(possibly after M3).

[nweiz, 2013-02-13 00:13:56]

A few suggestions, but LGTM.

https://codereview.chromium.org/12208138/diff/2001/utils/pub/io.dart
File utils/pub/io.dart (left):

https://codereview.chromium.org/12208138/diff/2001/utils/pub/io.dart#oldcode318
utils/pub/io.dart:318: log.fine("Create symlink $from -> $to.");
The fact that this plausibly makes sense both ways indicates that maybe it
should be made more explicit. Maybe "Creating symlink ($to is a symlink to
$from)".

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart
File utils/pub/path_source.dart (right):

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:19: /// A package [Source] that installs packages
from a given local file path.
Add a TODO here about supporting relative paths.

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:49: String _validatePath(String name, String dir) {
It seems like this should throw an error, which can be caught in install().

Also it seems like it should be a FormatError.

Why isn't this part of validateDescription?

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:61: "Was '$dir'.";
Don't we usually indent continued strings four spaces? Are we changing our
style?

https://codereview.chromium.org/12208138/diff/2001/utils/tests/pub/install/pa...
File utils/tests/pub/install/path/absolute_path_test.dart (right):

https://codereview.chromium.org/12208138/diff/2001/utils/tests/pub/install/pa...
utils/tests/pub/install/path/absolute_path_test.dart:21: "foo": {"path":
path.join(sandboxDir, "foo") }
Style nit: no space between ")" and "}".

[Bob Nystrom, 2013-02-13 18:28:36]

Thanks!

https://codereview.chromium.org/12208138/diff/2001/utils/pub/io.dart
File utils/pub/io.dart (left):

https://codereview.chromium.org/12208138/diff/2001/utils/pub/io.dart#oldcode318
utils/pub/io.dart:318: log.fine("Create symlink $from -> $to.");
On 2013/02/13 00:13:56, nweiz wrote:
> The fact that this plausibly makes sense both ways indicates that maybe it
> should be made more explicit. Maybe "Creating symlink ($to is a symlink to
> $from)".

Done.

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart
File utils/pub/path_source.dart (right):

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:19: /// A package [Source] that installs packages
from a given local file path.
On 2013/02/13 00:13:56, nweiz wrote:
> Add a TODO here about supporting relative paths.

Done.

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:49: String _validatePath(String name, String dir) {
On 2013/02/13 00:13:56, nweiz wrote:
> It seems like this should throw an error, which can be caught in install().

Done.

> Also it seems like it should be a FormatError.

Done.

> Why isn't this part of validateDescription?

Doing it there is a really blunt instrument. That gets called when a pubspec is
being parsed, which means pub wouldn't even load the root package if you have a
path dep to a non-existent file.

https://codereview.chromium.org/12208138/diff/2001/utils/pub/path_source.dart...
utils/pub/path_source.dart:61: "Was '$dir'.";
On 2013/02/13 00:13:56, nweiz wrote:
> Don't we usually indent continued strings four spaces? Are we changing our
> style?

Fixed. We do line up continued strings inside function calls often, like:

blahBlahBlah("some string that's really long "
             "here's the next line");

But we don't do that when it's just in a bare statement. Fixed as a side-effect
of throwing FormatException here.

https://codereview.chromium.org/12208138/diff/2001/utils/tests/pub/install/pa...
File utils/tests/pub/install/path/absolute_path_test.dart (right):

https://codereview.chromium.org/12208138/diff/2001/utils/tests/pub/install/pa...
utils/tests/pub/install/path/absolute_path_test.dart:21: "foo": {"path":
path.join(sandboxDir, "foo") }
On 2013/02/13 00:13:56, nweiz wrote:
> Style nit: no space between ")" and "}".

Done.