Add a new --extra-lib option to apidoc that allows us to specify an explicit .dart file that should…

Add a new --extra-lib option to apidoc that allows us to specify an explicit .dart file that should be included in the processing.
Change the gyp file to use --extra-lib for mock.dart.
Remove the import of mock.dart from unittest.dart.

R=amouravski@google.com

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

[gram, 2013-05-07 21:32:01]

If there is anyone else you think should be a reviewer let me know.

[Andrei Mouravski, 2013-05-08 10:43:47]

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (right):

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart#newc...
utils/apidoc/apidoc.dart:42: List<String> includedLibraries = <String>[];
You need to be more clear about this. Why is includedLibraries not good enough
for your use case?

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart#newc...
utils/apidoc/apidoc.dart:146: // Add any --extra libraries that had full pkg
paths.
This feels really hacky, so please add a TODO here to unhackify this.

[gram, 2013-05-08 22:14:36]

PTAL

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (right):

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart#newc...
utils/apidoc/apidoc.dart:42: List<String> includedLibraries = <String>[];
On 2013/05/08 10:43:47, Andrei Mouravski wrote:
> You need to be more clear about this. Why is includedLibraries not good enough
> for your use case?

I added some comments in the code, but the long story is below:

The extra libraries are put in includedLibraries eventually, but there is
different logic being used for these, which is why, despite the gyp file
currently having --include-lib=mock.dart, it wasn't working before. We need some
collection to gather the arguments passed by --extra-lib unless we want to
process each one in place, which is not how any of the other similar flags are
being processed. Each of --include-lib and --exclude-lib have a collection to
gather these during argument processing, and I am being consistent with that
model.

include-lib currently only works for dart files whose names match the package
name - so unittest/lib/unittest.dart will work, but unittest/lib/matcher.dart
will not (similarly for mock.dart, vm_configuration.dart, etc etc). Because the
include-lib files are specified just with a filename <foo>.dart and then a check
is done for pkg/<foo>lib/<foo>.dart, we can't handle any other case in the
existing code. 

That leaves several options:

- leave the flag as is and add some processing where we search pkg recursively
for all files matching pkg/*/lib/<foo>.dart and add these; I don't like this as
it can violate the principle of least surprise
- change --include-lib to expect a full path in pkg like
--include-lib=unittest/lib/unittest.dart; I'm okay with that as an alternative
- change include-lib to handle either case, so we can do
--include-lib=unittest.dart --include-lib=unittest/lib/mock.dart; that's just
ugly
- leave --include-lib processing as it is, and add a different flag to handle
the <foo>/lib/<bar>.dart case, which to me is the cleanest and best solution and
the approach I took.

https://codereview.chromium.org/14736010/diff/1/utils/apidoc/apidoc.dart#newc...
utils/apidoc/apidoc.dart:146: // Add any --extra libraries that had full pkg
paths.
On 2013/05/08 10:43:47, Andrei Mouravski wrote:
> This feels really hacky, so please add a TODO here to unhackify this.

Done.

[Andrei Mouravski, 2013-05-09 18:51:36]

lgtm

[gram, 2013-05-09 20:00:30]

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