Returning the closest public superclass rather than returning ""

Returning the closest public superclass rather than returning ""

BUG=
R=rnystrom@google.com

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

[Bob Nystrom, 2013-08-01 23:31:40]

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

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:628: if (superclass == null) return "";
Is there a reason to return "" instead of "Object" here?

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:629: else {
"else" isn't needed here.

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:630: if (_filterPrivate(superclass)) return
superclass.qualifiedName;
"filter" is a bit confusing here. I would expect it to return true if superclass
*is* private. How about "_isHidden"?

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:631: else return superclass.validSuperclass();
"else" isn't needed here.

[janicejl, 2013-08-02 00:49:09]

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

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:628: if (superclass == null) return "";
On 2013/08/01 23:31:40, Bob Nystrom wrote:
> Is there a reason to return "" instead of "Object" here?

Done.

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:629: else {
On 2013/08/01 23:31:40, Bob Nystrom wrote:
> "else" isn't needed here.

Done.

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:630: if (_filterPrivate(superclass)) return
superclass.qualifiedName;
On 2013/08/01 23:31:40, Bob Nystrom wrote:
> "filter" is a bit confusing here. I would expect it to return true if
superclass
> *is* private. How about "_isHidden"?

Done.

https://codereview.chromium.org/20617005/diff/1/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:631: else return superclass.validSuperclass();
On 2013/08/01 23:31:40, Bob Nystrom wrote:
> "else" isn't needed here.

Done.

[Bob Nystrom, 2013-08-02 02:25:03]

Sorry, I told you the wrong name. A couple of more suggestions then LGTM after
that.

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

https://codereview.chromium.org/20617005/diff/6001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:282: bool _isHidden(Indexable item) {
Hmm, I think I told you to name this backwards. It returns true if the item
should be shown, right? Maybe _isVisible().

https://codereview.chromium.org/20617005/diff/6001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:639: 'implements': new List.from(interfaces.where((e)
=> _isHidden(e))
Instead of where((e) => _isHidden(e)), you can just do: where(_isHidden)

https://codereview.chromium.org/20617005/diff/6001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:704: .where((e) => _isHidden(e)).map((e) =>
e.qualifiedName)),
And here too:

where(_isHidden)

[janicejl, 2013-08-02 02:50:39]

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