dart:io | Add 'recursive' flag to File.create and Link.create.

dart:io | Add 'recursive' flag to File.create and Link.create.

BUG=dartbug.com/12462
R=ajohnsen@google.com

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

[Anders Johnsen, 2013-10-30 12:23:55]

LGTM!

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart
File sdk/lib/io/file.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart#newc...
sdk/lib/io/file.dart:49: * components are created.
Default is `false`.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart#newc...
sdk/lib/io/file.dart:66: * components are created.
Ditto.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart
File sdk/lib/io/file_impl.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart...
sdk/lib/io/file_impl.dart:249: .then((_) => recursive ? parent.exists() : true)
Any reason to call parent.exists()? It should do it automaticly in
parent.create. What about

  .then((_) => recursive ? parent.create(recursive: true) : true)
  .then((_) => _IOService.dispatch(_FILE_CREATE, [path]))

?

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart...
sdk/lib/io/file_impl.dart:267: if (recursive && !parent.existsSync()) {
Ditto.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart
File sdk/lib/io/link.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart#newc...
sdk/lib/io/link.dart:163: .then((_) => recursive ? parent.exists() : true)
Ditto.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart#newc...
sdk/lib/io/link.dart:176: if (recursive && !parent.existsSync()) {
Ditto.

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/creat...
File tests/standalone/io/create_recursive_test.dart (right):

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/creat...
tests/standalone/io/create_recursive_test.dart:16: testCreateRecursiveSync() {
Can you split up these two functions to smaller tests?

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/direc...
File tests/standalone/io/directory_test.dart (left):

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/direc...
tests/standalone/io/directory_test.dart:597: testCreateRecursiveSync() {
These tests could be copied directory to the new file.

[Bill Hesse, 2013-10-30 15:07:36]

Much better looking, thanks.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart
File sdk/lib/io/file.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart#newc...
sdk/lib/io/file.dart:49: * components are created.
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Default is `false`.

Done.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file.dart#newc...
sdk/lib/io/file.dart:66: * components are created.
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Ditto.

Done.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart
File sdk/lib/io/file_impl.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart...
sdk/lib/io/file_impl.dart:249: .then((_) => recursive ? parent.exists() : true)
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Any reason to call parent.exists()? It should do it automaticly in
> parent.create. What about
> 
>   .then((_) => recursive ? parent.create(recursive: true) : true)
>   .then((_) => _IOService.dispatch(_FILE_CREATE, [path]))
> 
> ?

Done.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/file_impl.dart...
sdk/lib/io/file_impl.dart:267: if (recursive && !parent.existsSync()) {
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Ditto.

Done.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart
File sdk/lib/io/link.dart (right):

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart#newc...
sdk/lib/io/link.dart:163: .then((_) => recursive ? parent.exists() : true)
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Ditto.

Done.

https://codereview.chromium.org/52363002/diff/30001/sdk/lib/io/link.dart#newc...
sdk/lib/io/link.dart:176: if (recursive && !parent.existsSync()) {
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Ditto.

Done.

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/creat...
File tests/standalone/io/create_recursive_test.dart (right):

https://codereview.chromium.org/52363002/diff/30001/tests/standalone/io/creat...
tests/standalone/io/create_recursive_test.dart:16: testCreateRecursiveSync() {
On 2013/10/30 12:23:56, Anders Johnsen wrote:
> Can you split up these two functions to smaller tests?

Done.

[Anders Johnsen, 2013-10-30 16:19:12]

LGTM, much better!

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
File sdk/lib/io/directory_impl.dart (right):

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
sdk/lib/io/directory_impl.dart:93: Future<Directory> create({bool recursive:
false}) {
Yay, awesome recursion :)

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
sdk/lib/io/directory_impl.dart:97: if (path != parent.path) {
Can we start with the path != parent.path, that is, before the exists test?

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
sdk/lib/io/directory_impl.dart:102: return create();
I suppose this will always fail, as this is us trying to create a root
directory?

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
File tests/standalone/io/create_recursive_test.dart (right):

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
tests/standalone/io/create_recursive_test.dart:128: .then((_) =>
link.create(temp.path, recursive: true))
indentation

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
tests/standalone/io/create_recursive_test.dart:135: () => link.create(temp.path,
recursive: true),
indentation

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
tests/standalone/io/create_recursive_test.dart:139: .then((_) => asyncEnd())
asyncEnd should be part of whenComplete (likewise in the other async tests).

[Bill Hesse, 2013-10-30 16:32:06]

Committed patchset #4 manually as r29563 (presubmit successful).

[Bill Hesse, 2013-10-30 16:51:40]

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
File sdk/lib/io/directory_impl.dart (right):

https://codereview.chromium.org/52363002/diff/140001/sdk/lib/io/directory_imp...
sdk/lib/io/directory_impl.dart:97: if (path != parent.path) {
On 2013/10/30 16:19:13, Anders Johnsen wrote:
> Can we start with the path != parent.path, that is, before the exists test?

The path != parent.path should almost never be true.  We would have to have a
root not existing, as you point out.  And maybe we shouldn't even try to call
create, but that does return the most appropriate error.

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
File tests/standalone/io/create_recursive_test.dart (right):

https://codereview.chromium.org/52363002/diff/140001/tests/standalone/io/crea...
tests/standalone/io/create_recursive_test.dart:135: () => link.create(temp.path,
recursive: true),
On 2013/10/30 16:19:13, Anders Johnsen wrote:
> indentation

This was intentional. An indentation over 4 spaces that lines things up is OK. 
And this is 4 spaces more than the expression that is the target of a =>.  But I
changed it to a more standard one.