Make listDir and createSymlink synchronous in pub.

Make listDir and createSymlink synchronous in pub.

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

[Bob Nystrom, 2013-03-29 21:22:06]

https://codereview.chromium.org/13332009/diff/1/utils/pub/command_lish.dart
File utils/pub/command_lish.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/command_lish.dart#n...
utils/pub/command_lish.dart:152: }).then((files) =>
files.where(_shouldPublish).toList());
Ditch this and tack the .where() and .toList() to the previous line.

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart
File utils/pub/entrypoint.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:251: if (dirExists(binDir))
_linkSecondaryPackageDir(binDir);
We can actually use a loop here now:

for (var name in ['example', 'test', 'tool', 'web']) {
  _linkSecondaryPackageDirsRecursively(path.join(root.dir, name));
}

(Or map(), forEach(), whatevs.)

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:261: print("linking $dir recursively");
Debug code.

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:285: print("linking $dir");
Debug.

https://codereview.chromium.org/13332009/diff/1/utils/pub/hosted_source.dart
File utils/pub/hosted_source.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/hosted_source.dart#...
utils/pub/hosted_source.dart:121: Future<List<Package>> getCachedPackages() {
Make this sync too.

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode129
utils/pub/io.dart:129: /// contents (defaults to `false`). If
[includeHiddenFiles] is `true`, includes
includeHiddenFiles -> includeHidden

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode151
utils/pub/io.dart:151: if (!includeHiddenFiles &&
path.basename(file).startsWith('.')) {
These can probably be one-liners if you shorten it to includeHidden

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode238
utils/pub/io.dart:238: new Link(symlink).createSync(target);
Nice!

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode620
utils/pub/io.dart:620: var tarFile = listDir(tempDir).firstWhere((file) =>
Nit, but how about moving (file) => to the next line?

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
File utils/pub/validator/compiled_dartdoc.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
utils/pub/validator/compiled_dartdoc.dart:23: return new Future.of(() {
If you you're going to switch some defer() calls to Future.of(), might as well
do them all.

[nweiz, 2013-03-29 22:09:02]

https://codereview.chromium.org/13332009/diff/1/utils/pub/command_lish.dart
File utils/pub/command_lish.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/command_lish.dart#n...
utils/pub/command_lish.dart:152: }).then((files) =>
files.where(_shouldPublish).toList());
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> Ditch this and tack the .where() and .toList() to the previous line.

We need this to catch the case where the files come from git.

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart
File utils/pub/entrypoint.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:251: if (dirExists(binDir))
_linkSecondaryPackageDir(binDir);
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> We can actually use a loop here now:
> 
> for (var name in ['example', 'test', 'tool', 'web']) {
>   _linkSecondaryPackageDirsRecursively(path.join(root.dir, name));
> }
> 
> (Or map(), forEach(), whatevs.)

Done.

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:261: print("linking $dir recursively");
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> Debug code.

Done.

https://codereview.chromium.org/13332009/diff/1/utils/pub/entrypoint.dart#new...
utils/pub/entrypoint.dart:285: print("linking $dir");
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> Debug.

Done.

https://codereview.chromium.org/13332009/diff/1/utils/pub/hosted_source.dart
File utils/pub/hosted_source.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/hosted_source.dart#...
utils/pub/hosted_source.dart:121: Future<List<Package>> getCachedPackages() {
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> Make this sync too.

Done, although we may have to move it back to async in the future if some source
needs to do some async operations here.

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode129
utils/pub/io.dart:129: /// contents (defaults to `false`). If
[includeHiddenFiles] is `true`, includes
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> includeHiddenFiles -> includeHidden

That's out of scope of this change, but I'll add a TODO.

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode151
utils/pub/io.dart:151: if (!includeHiddenFiles &&
path.basename(file).startsWith('.')) {
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> These can probably be one-liners if you shorten it to includeHidden

See above.

https://codereview.chromium.org/13332009/diff/1/utils/pub/io.dart#newcode620
utils/pub/io.dart:620: var tarFile = listDir(tempDir).firstWhere((file) =>
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> Nit, but how about moving (file) => to the next line?

Done.

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
File utils/pub/validator/compiled_dartdoc.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
utils/pub/validator/compiled_dartdoc.dart:23: return new Future.of(() {
On 2013/03/29 21:22:06, Bob Nystrom wrote:
> If you you're going to switch some defer() calls to Future.of(), might as well
> do them all.

I'll do it in a future CL.

[Bob Nystrom, 2013-03-29 22:14:55]

LGTM.

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
File utils/pub/validator/compiled_dartdoc.dart (right):

https://codereview.chromium.org/13332009/diff/1/utils/pub/validator/compiled_...
utils/pub/validator/compiled_dartdoc.dart:23: return new Future.of(() {
On 2013/03/29 22:09:02, nweiz wrote:
> On 2013/03/29 21:22:06, Bob Nystrom wrote:
> > If you you're going to switch some defer() calls to Future.of(), might as
well
> > do them all.
> 
> I'll do it in a future CL.

OK, add a TODO where defer() is defined then.

[nweiz, 2013-03-29 22:20:31]

Committed patchset #2 manually as r20698 (presubmit successful).