Change to use Pathos

finds packageDir from the returned list from listDir. 

used pathos package and moved to listDir from pub/lib/src/io.dart

moved listLibraries into the docgen class, and changed return type to Strings.

R=amouravski@google.com

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

[Emily Fortuna, 2013-06-25 02:06:24]

can you please first rebase this on to https://codereview.chromium.org/17608007/
so we can see what actually changed?

[janicejl, 2013-06-25 17:32:44]

Have rebased. 

Thank you.

[Andrei Mouravski, 2013-06-25 18:37:41]

https://chromiumcodereview.appspot.com/17611006/diff/4001/pkg/docgen/lib/docg...
File pkg/docgen/lib/docgen.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/4001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:16: 
You don't need this newline.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
File pkg/docgen/lib/docgen.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:62: /// Package Directory of
Of what? don't leave me in suspense!

Also, comments should start with a capital and end with a period. Fix the one
above?

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:100: if (args.length != 1) {
Remove the curlies and put this on one line.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:106: if (type == FileSystemEntityType.NOT_FOUND) {
How about you create an "else" for this whole big if else if stuff and move the
exception into the else block.

That way, you don't need to special case NOT_FOUND.

Like, what if type is null?

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:109: var files = listDir(resolveLink(args[0]),
recursive: true);
Split the first else if block into a separate function.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:114: packageDir = packageDir == "" ?
Rewrite this as:
if (packageDir != '') packageDir = path.dirname(packageDir) + '/packages';

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:115: "" : path.dirname(packageDir) + "/packages";
Change all " to '

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:120: var files = listDir(args[0], recursive: true);
This is copypasta from above. Another good reason to pull out the function (use
the editor for this!)

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:129: libraries.forEach((lib) => logger.info("Added to
libraries: $lib"));
You should be logging this as you add each library to libraries in case one of
them breaks, you know exactly which one it was.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:139: if (libraries.isEmpty) throw new
UnsupportedError("No Libraries. ");
Why is there a space after the period?

Also, make sure all " are '

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:152: packageRoot: new
Path(packageDir)).then((MirrorSystem mirrorSystem) {
Put the ".then" on the next line. Also, don't call new Path here. Do it inside
of getMirrorSystem.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:166: Path libraryRoot, {Path packageRoot}) {
Take strings instead of Paths.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/io.dart
File pkg/docgen/lib/io.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/io.d...
pkg/docgen/lib/io.dart:1: library io;
This file should probably be in lib/src

Also, add doc comment to library.

Also add a NON-doc comment here to say where you copy pasta'd it from.

[janicejl, 2013-06-25 19:47:49]

https://chromiumcodereview.appspot.com/17611006/diff/4001/pkg/docgen/lib/docg...
File pkg/docgen/lib/docgen.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/4001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:16: 
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> You don't need this newline.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
File pkg/docgen/lib/docgen.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:62: /// Package Directory of
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Of what? don't leave me in suspense!
> 
> Also, comments should start with a capital and end with a period. Fix the one
> above?

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:100: if (args.length != 1) {
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Remove the curlies and put this on one line.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:106: if (type == FileSystemEntityType.NOT_FOUND) {
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> How about you create an "else" for this whole big if else if stuff and move
the
> exception into the else block.
> 
> That way, you don't need to special case NOT_FOUND.
> 
> Like, what if type is null?

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:109: var files = listDir(resolveLink(args[0]),
recursive: true);
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Split the first else if block into a separate function.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:114: packageDir = packageDir == "" ?
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Rewrite this as:
> if (packageDir != '') packageDir = path.dirname(packageDir) + '/packages';

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:115: "" : path.dirname(packageDir) + "/packages";
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Change all " to '

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:120: var files = listDir(args[0], recursive: true);
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> This is copypasta from above. Another good reason to pull out the function
(use
> the editor for this!)

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:129: libraries.forEach((lib) => logger.info("Added to
libraries: $lib"));
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> You should be logging this as you add each library to libraries in case one of
> them breaks, you know exactly which one it was.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:139: if (libraries.isEmpty) throw new
UnsupportedError("No Libraries. ");
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Why is there a space after the period?
> 
> Also, make sure all " are '

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:152: packageRoot: new
Path(packageDir)).then((MirrorSystem mirrorSystem) {
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Put the ".then" on the next line. Also, don't call new Path here. Do it inside
> of getMirrorSystem.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/docg...
pkg/docgen/lib/docgen.dart:166: Path libraryRoot, {Path packageRoot}) {
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> Take strings instead of Paths.

Done.

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/io.dart
File pkg/docgen/lib/io.dart (right):

https://chromiumcodereview.appspot.com/17611006/diff/7001/pkg/docgen/lib/io.d...
pkg/docgen/lib/io.dart:1: library io;
On 2013/06/25 18:37:41, Andrei Mouravski wrote:
> This file should probably be in lib/src
> 
> Also, add doc comment to library.
> 
> Also add a NON-doc comment here to say where you copy pasta'd it from.

Done.

[Andrei Mouravski, 2013-06-25 20:13:22]

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:58: /// Resolves reference links in doc-comments.
"doc comments."

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:61: /// Package Directory of directory being
analyzed.
Directory -> directory.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:99: if (args.length != 1) throw new
UnsupportedError(usage);
Add a TODO here as to why it needs to be this way.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:135: if (libraries.isEmpty) throw new
UnsupportedError('No Libraries.');
Because you check isEmpty here, you don't need to worry about listLibraries
returning nothing.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:135: if (libraries.isEmpty) throw new
UnsupportedError('No Libraries.');
Make a more descriptive error.
Also, it's possibly a stateError?

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:162: String libraryRoot, {String packageRoot}) {
Don't forget to actually update libraryRoot to Paths maybe?

[janicejl, 2013-06-25 20:50:15]

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:58: /// Resolves reference links in doc-comments.
On 2013/06/25 20:13:22, Andrei Mouravski wrote:
> "doc comments."

Done.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:61: /// Package Directory of directory being
analyzed.
On 2013/06/25 20:13:22, Andrei Mouravski wrote:
> Directory -> directory.

Done.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:99: if (args.length != 1) throw new
UnsupportedError(usage);
On 2013/06/25 20:13:22, Andrei Mouravski wrote:
> Add a TODO here as to why it needs to be this way.

Done.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:135: if (libraries.isEmpty) throw new
UnsupportedError('No Libraries.');
On 2013/06/25 20:13:22, Andrei Mouravski wrote:
> Because you check isEmpty here, you don't need to worry about listLibraries
> returning nothing.

Done.

https://codereview.chromium.org/17611006/diff/17001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:162: String libraryRoot, {String packageRoot}) {
On 2013/06/25 20:13:22, Andrei Mouravski wrote:
> Don't forget to actually update libraryRoot to Paths maybe?

Should I still make it a Path if it works without changing it to a path?

[Andrei Mouravski, 2013-06-25 21:24:16]

lgtm

[janicejl, 2013-06-25 21:38:33]

Committed patchset #5 manually as r24434 (presubmit successful).