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

Issue 9256002: Add a mode to dartdoc to generate the navigation on the client. (Closed)

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

Description

Add a mode to dartdoc to generate the navigation on the client. Right now, the generated docs are pretty huge (100MB for all of the main libs). This is mainly because each generated HTML page includes full navigation for each type in the containing library. For stuff like dart:html, that's tons of HTML. Even after compression, the docs are 9.2MB. Also, they take a long time to generate (30s on my laptop). This patch adds a mode you can use called "live-nav" (and makes it the default). With that mode, pages do not include navigation. Instead, we generate a single nav.json file with the navigation data. Client-side Dart code does an XHR for that and then renders the navigation on the fly. This gets the generated docs down to 13.3MB (2MB after compression) and gets doc generation up to 6.3s for everything. The two downsides are: 1. There's a visible flash on page load when it renders the nav since it's asynchronous. 2. You can't browse docs locally just using file://. To be able to XHR the nav.json, you either need to enable file access in your browser or run a local server. Committed: https://code.google.com/p/dart/source/detail?r=3445

Patch Set 1 #

Total comments: 17

Patch Set 2 : Respond to review. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+287 lines, -97 lines) Patch
M utils/apidoc/html_diff.dart View 1 1 chunk +1 line, -36 lines 0 comments Download
M utils/dartdoc/.gitignore View 1 chunk +3 lines, -2 lines 0 comments Download
M utils/dartdoc/classify.dart View 1 chunk +5 lines, -0 lines 0 comments Download
A utils/dartdoc/client-live-nav.dart View 1 1 chunk +111 lines, -0 lines 0 comments Download
A utils/dartdoc/client-shared.dart View 1 chunk +33 lines, -0 lines 0 comments Download
A utils/dartdoc/client-static.dart View 1 1 chunk +18 lines, -0 lines 0 comments Download
M utils/dartdoc/dartdoc View 1 chunk +12 lines, -8 lines 0 comments Download
M utils/dartdoc/dartdoc.dart View 1 10 chunks +104 lines, -13 lines 0 comments Download
D utils/dartdoc/interact.dart View 1 chunk +0 lines, -38 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Bob Nystrom
I think this should be good enough to take care of: http://code.google.com/p/dart/issues/detail?id=785. Once the VM's ...
8 years, 11 months ago (2012-01-18 22:08:05 UTC) #1
nweiz
lgtm It might be nice to add some instructions on using tip/toss to serve these ...
8 years, 11 months ago (2012-01-19 19:20:17 UTC) #2
Bob Nystrom
8 years, 11 months ago (2012-01-19 20:11:07 UTC) #3
Thanks!

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
File utils/dartdoc/client-live-nav.dart (right):

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:28: currentType =
body.attributes['data-type'];
On 2012/01/19 19:20:17, nweiz wrote:
> body.dataAttributes['type'] is slightly more idiomatic.

Done.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:46: * a string of HTML representation the
navigation for those libraries, relative
On 2012/01/19 19:20:17, nweiz wrote:
> s/representation/representing/

Done. This comment was all kinds of out of date.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:51: libraryNames.sort((a, b) =>
a.compareTo(b));
On 2012/01/19 19:20:17, nweiz wrote:
> Off topic: this should really be the default argument to #sort.

Yeah. Filed a bug: dartbug.com/1235

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:56: if ((currentLibrary == libraryName) &&
(currentType == null)) {
On 2012/01/19 19:20:17, nweiz wrote:
> Style nit: redundant parens.

I usually don't rely on operator precedence, but for you I will. Done.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:57:
html.add('<strong>$libraryName</strong>');
On 2012/01/19 19:20:17, nweiz wrote:
> How sure are we that libraryName will never contain any HTML-active
characters?

Pretty sure, but sanitized just in case.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:82: if (type['name'].endsWith('Exception')) {
On 2012/01/19 19:20:17, nweiz wrote:
> This seems a little hacky. Could you include an isException flag in the JSON
> that's set based on whether the class extends Exception?

It is a bit hacky, but I'm OK with that at least for now. I put this in because
corelib has such a huge number of exception types that they were cluttering up
the regular types, but I'm not crazy about having to do this. I'd rather corelib
be cleaned up.

I kind of like going by name because it isn't necessary to extend Exception in
order to be throwable in Dart. I like the idea of having dartdoc go by naming
convention instead.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-li...
utils/dartdoc/client-live-nav.dart:89: if ((types.length == 0) &&
(exceptions.length == 0)) return;
On 2012/01/19 19:20:17, nweiz wrote:
> Style nit: more redundant parens.

Done.

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-st...
File utils/dartdoc/client-static.dart (right):

https://chromiumcodereview.appspot.com/9256002/diff/1/utils/dartdoc/client-st...
utils/dartdoc/client-static.dart:11: #import('markdown.dart', prefix: 'md');
On 2012/01/19 19:20:17, nweiz wrote:
> I don't think this is actually used.

Good catch!

Powered by Google App Engine
This is Rietveld 408576698