Expose exported classes as well.

Expose exported classes as well.

BUG=
R=alanknight@google.com

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

[Emily Fortuna, 2013-11-20 20:58:44]

Did some refactoring of docgen as well to use inheritance (!) and encapsulation
(!), but there is definitely more that can be done.

[Alan Knight, 2013-11-20 21:29:19]

LGTM, some nits and one leftover debug statement.

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

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:379: var result = new Library(library);
Awesome!

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:733: CommentGenerator get _commentFunction =>
If this is always there, could we just call make it a method (fetchComment or
similar) and call it directly?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:765: MethodGroup _processMethods(Map<String,
MethodMirror> mirrorMap) {
A bit of inconsistency with naming that we're processing methods but populating
exports. I think I like populating better, as process doesn't say much. Or
create?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:862: print(classes.toMap().toString());
Leftover debugging?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:881: /// Returns a map of [Class] objects constructed
from [mirrorMap].
"Returns a [ClassGroup] with a map of" ?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1058: /// Returns the [Class] for the given [mirror]
has already been created, and
Missing an "if it"?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1060: static Class
_getClassFromClassMirror(ClassMirror mirror) {
Naming nit. The "get" doesn't add much, and if it's a static on Class that
returns Class, couldn't it just be fromClassMirror(). Or even fromMirror()? And
maybe should be a factory constructor?

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1233: String get name => this._variableName;
Why the explicit "this."?

[Emily Fortuna, 2013-11-21 19:26:32]

Committed patchset #1 manually as r30542 (presubmit successful).

[Emily Fortuna, 2013-11-21 19:29:58]

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

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:733: CommentGenerator get _commentFunction =>
On 2013/11/20 21:29:20, Alan Knight wrote:
> If this is always there, could we just call make it a method (fetchComment or
> similar) and call it directly?

Good point. I ended up doing this refactor in incremental pieces because I ran
into some surprising side effects along the way, but I missed some
macro-refactors as a result. Fixed!

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:765: MethodGroup _processMethods(Map<String,
MethodMirror> mirrorMap) {
On 2013/11/20 21:29:20, Alan Knight wrote:
> A bit of inconsistency with naming that we're processing methods but
populating
> exports. I think I like populating better, as process doesn't say much. Or
> create?

renamed to create

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:862: print(classes.toMap().toString());
On 2013/11/20 21:29:20, Alan Knight wrote:
> Leftover debugging?

yep. removed.

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:881: /// Returns a map of [Class] objects constructed
from [mirrorMap].
On 2013/11/20 21:29:20, Alan Knight wrote:
> "Returns a [ClassGroup] with a map of" ?

Done.

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1058: /// Returns the [Class] for the given [mirror]
has already been created, and
On 2013/11/20 21:29:20, Alan Knight wrote:
> Missing an "if it"?

Done.

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1060: static Class
_getClassFromClassMirror(ClassMirror mirror) {
On 2013/11/20 21:29:20, Alan Knight wrote:
> Naming nit. The "get" doesn't add much, and if it's a static on Class that
> returns Class, couldn't it just be fromClassMirror(). Or even fromMirror()?
And
> maybe should be a factory constructor?

Good call. Fixed.

https://codereview.chromium.org/78713004/diff/80001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:1233: String get name => this._variableName;
On 2013/11/20 21:29:20, Alan Knight wrote:
> Why the explicit "this."?

oops. Refactoring cruft from when it was part of the constructor.
Fixed.