Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issue 9146016: Add the ability to link to members and constructors of other classes in Dartdoc. (Closed)

Created:
8 years, 11 months ago by nweiz
Modified:
8 years, 11 months ago
Reviewers:
Bob Nystrom
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Add the ability to link to members and constructors of other classes in Dartdoc. The syntax is [Class.member] and [new Class], respectively. [new Class.namedConstructor] also works. Committed: https://code.google.com/p/dart/source/detail?r=3217

Patch Set 1 #

Total comments: 7

Patch Set 2 : Code review changes #

Patch Set 3 : Merge with head #

Patch Set 4 : Get tests running again. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -6 lines) Patch
M utils/dartdoc/dartdoc.dart View 1 2 3 2 chunks +31 lines, -5 lines 0 comments Download
M utils/dartdoc/files.dart View 1 2 3 1 chunk +6 lines, -1 line 0 comments Download
M utils/tests/dartdoc/src/dartdoc_tests.dart View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
M utils/tests/dartdoc/src/dummy.dart View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
nweiz
I've confirmed that static members share a namespace with instance members, so [Class.member] should work ...
8 years, 11 months ago (2012-01-09 23:50:07 UTC) #1
Bob Nystrom
A couple of suggestions, but LGTM. http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart File utils/dartdoc/dartdoc.dart (right): http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart#newcode794 utils/dartdoc/dartdoc.dart:794: findMember(Type type, String ...
8 years, 11 months ago (2012-01-10 00:40:41 UTC) #2
nweiz
8 years, 11 months ago (2012-01-10 01:51:55 UTC) #3
http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart
File utils/dartdoc/dartdoc.dart (right):

http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:794: findMember(Type type, String name) {
On 2012/01/10 00:40:41, Bob Nystrom wrote:
> Shadowing name here, makes this already-confusing method even more confusing.
> Can you rename one of them?

Done.

http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:833: final classAndName =
name.substring(4).split('.');
On 2012/01/10 00:40:41, Bob Nystrom wrote:
> I think a regex would be simpler here: 'new (\w+).(\w+)'.

Done.

http://codereview.chromium.org/9146016/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:841: })();
On 2012/01/10 00:40:41, Bob Nystrom wrote:
> This is clever, but perverse. :) If using a regex simplifies your control flow
> enough that you don't need all of the early returns, I'd try to get rid of
this.
> Otherwise, maybe just make this a standalone method, or a local named function
> that you call after defining.

I had this as a local named function before this, but it felt spaghetti-esque
when reading the code, what with having to jump backwards to figure out what the
function was doing. I think this approach better captures the control flow I'm
trying to express, despite being somewhat unconventional.

Powered by Google App Engine
This is Rietveld 408576698