Add a script to generate HTML and DOM docs with cross-links to one another.

Add a script to generate HTML and DOM docs with cross-links to one another.

This entails adding callbacks to dartdoc for injecting additional
comments, as well as adding somewhat deeper support for hashing and
equality testing of Frog objects.

The next step is to inject the annotations into the HTML and DOM library source itself.

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

[Jacob, 2011-12-06 22:38:14]

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.dart
File client/html/scripts/html_doc.dart (right):

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:32: String _andList(List<String> items) {
nit: name _andList is a bit confusing.  Maybe something like
listToAndSeparatedString
?

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:65: var htmlMemberSet =
_mergeProps(_diff.domToHtml[member]);
nit: var ==> final

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:76: return "_This $phrase ${domMembers} in the
[dart:dom](../dom.html) " +
nit: personally i'd recommend violating the 80 char line limit instead of using
" + "

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
File client/html/src/ElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
client/html/src/ElementWrappingImplementation.dart:575: *   children
and 
appendChild

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
client/html/src/ElementWrappingImplementation.dart:699: Element query(String
selectors) {
@domName querySelector, getElementById

[Bob Nystrom, 2011-12-06 23:11:23]

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.dart
File client/html/scripts/html_doc.dart (right):

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:6: * A script to document the HTML library.
This needs to be run from the
To document what about the HTML library? For a script about documentation, it's
sadly devoid of it. :(

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:7: * utils/dartdoc directory.
I'd move this into dartdoc. I know it's specific to HTML, but dartdoc already
has a little special sauce for HTML, and HTML is built into Dart.

Even more awesome would be to add a little shell script (htmldoc) that invokes
it.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:25: process.argv[2] = 'dart:html';
This is pretty gross. Right now dartdoc isn't set up to be run as a lib, but it
should be. Can you do that? The markdown lib might be a useful reference.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:32: String _andList(List<String> items) {
Dartdoc also has code that does this. Since you're already importing dartdoc,
refactor that out into a separate method in dartdoc and just call it here?

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:49: return
"[$memberName](../${doc.memberUrl(member)})";
Yay markdown!

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:55: // Only print setters if the getter
doesn't exist.
Make this a doc comment. I'd name this something descriptive: _unifyProperties?

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:66: var phrase = htmlMemberSet.every((m) =>
_diff.sameName(member, m)) ?
Some temporary variables would help here:

final blah = htmlMemberSet.every((m) => _diff.sameName(member, m))
final phrase = blah ? 'available as' : 'renamed to';

It isn't obvious what the result of .every() represents.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:76: return "_This $phrase ${domMembers} in the
[dart:dom](../dom.html) " +
Don't need {} on domMembers.

http://codereview.chromium.org/8771054/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8771054/diff/1/frog/member.dart#newcode199
frog/member.dart:199: int hashCode() =>
I respect your desire to use =>, but this is a bit gratuitous.

http://codereview.chromium.org/8771054/diff/1/frog/member.dart#newcode203
frog/member.dart:203: bool operator ==(other) => other is Member &&
This too. :)

http://codereview.chromium.org/8771054/diff/1/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/1/frog/type.dart#newcode386
frog/type.dart:386: // TODO(nweiz): make hashCode and == more robust by
By what? Don't leave me in suspense!

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart
File utils/dartdoc/dartdoc.dart (right):

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:61: List<DocTypeFunction> _docTypeFns;
How about "Documenter" instead of "Function" and "Fn" here and elsewhere?

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:259: String _callbacks(var item, List<Function>
callbacks) =>
Just because you *can* use a functional style doesn't mean you *must*. :)

Personally, I don't use => if I have to wrap to the next line, even though that
does mean needing a "return". Like it or not, Dart is statement-oriented and I
don't think we should fight that that hard.

Also, this name doesn't really illuminate. Maybe _runDocumenters?

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:653: docCode(SourceSpan span, String extraDocs, [bool
showCode = false]) {
"extraMarkdown"

We need to make it clear that this happens before md->html.

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:782: registerTypeCallback(DocTypeFunction fn) =>
_docTypeFns.add(fn);
I'd name this "add" instead of "register" to make it clear that it doesn't
replace a previously added one. Also, it isn't really clear what this callback
does for the type.

Maybe addTypeDocumenter(TypeDocumenter documenter)?

[nweiz, 2011-12-07 19:26:56]

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.dart
File client/html/scripts/html_doc.dart (right):

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:7: * utils/dartdoc directory.
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> I'd move this into dartdoc. I know it's specific to HTML, but dartdoc already
> has a little special sauce for HTML, and HTML is built into Dart.
> 
> Even more awesome would be to add a little shell script (htmldoc) that invokes
> it. 

Moving this into dartdoc runs into the same problem that we had with
unittest_node. This would transitively depend on ../markdown/ast.dart, as well
as on ../../client/html/scripts/html_diff.dart; but the latter would also depend
on ../../../utils/markdown/ast.dart. Frog doesn't recognize that the two
ast.darts are the same, which causes errors.

I've added the htmldoc wrapper script, though.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:25: process.argv[2] = 'dart:html';
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> This is pretty gross. Right now dartdoc isn't set up to be run as a lib, but
it
> should be. Can you do that? The markdown lib might be a useful reference.

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:32: String _andList(List<String> items) {
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> Dartdoc also has code that does this. Since you're already importing dartdoc,
> refactor that out into a separate method in dartdoc and just call it here?

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:55: // Only print setters if the getter
doesn't exist.
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> Make this a doc comment. I'd name this something descriptive:
_unifyProperties?

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:65: var htmlMemberSet =
_mergeProps(_diff.domToHtml[member]);
On 2011/12/06 22:38:14, Jacob wrote:
> nit: var ==> final

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:66: var phrase = htmlMemberSet.every((m) =>
_diff.sameName(member, m)) ?
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> Some temporary variables would help here:
> 
> final blah = htmlMemberSet.every((m) => _diff.sameName(member, m))
> final phrase = blah ? 'available as' : 'renamed to';
> 
> It isn't obvious what the result of .every() represents.

I think it reads pretty well... "if every member has the same name as [member],
...". But I guess I wrote it, so it's probably clearer to me than to a reader.
I'll change it.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:76: return "_This $phrase ${domMembers} in the
[dart:dom](../dom.html) " +
On 2011/12/06 22:38:14, Jacob wrote:
> nit: personally i'd recommend violating the 80 char line limit instead of
using
> " + "

That makes it harder to read in text editors that don't wrap lines (like mine).
It's also contrary to Google style, at least for JS.

http://codereview.chromium.org/8771054/diff/1/client/html/scripts/html_doc.da...
client/html/scripts/html_doc.dart:76: return "_This $phrase ${domMembers} in the
[dart:dom](../dom.html) " +
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> Don't need {} on domMembers.

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
File client/html/src/ElementWrappingImplementation.dart (right):

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
client/html/src/ElementWrappingImplementation.dart:575: *   children
On 2011/12/06 22:38:14, Jacob wrote:
> and 
> appendChild

Done.

http://codereview.chromium.org/8771054/diff/1/client/html/src/ElementWrapping...
client/html/src/ElementWrappingImplementation.dart:699: Element query(String
selectors) {
On 2011/12/06 22:38:14, Jacob wrote:
> @domName querySelector, getElementById

Done.

http://codereview.chromium.org/8771054/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8771054/diff/1/frog/member.dart#newcode199
frog/member.dart:199: int hashCode() =>
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> I respect your desire to use =>, but this is a bit gratuitous.

Done.

http://codereview.chromium.org/8771054/diff/1/frog/member.dart#newcode203
frog/member.dart:203: bool operator ==(other) => other is Member &&
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> This too. :)

Done, although it's still just one expression.

http://codereview.chromium.org/8771054/diff/1/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/1/frog/type.dart#newcode386
frog/type.dart:386: // TODO(nweiz): make hashCode and == more robust by
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> By what? Don't leave me in suspense!

It was going to be "adding Library#hashCode", but I decided to just add it
instead. TODO removed.

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart
File utils/dartdoc/dartdoc.dart (right):

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:61: List<DocTypeFunction> _docTypeFns;
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> How about "Documenter" instead of "Function" and "Fn" here and elsewhere?

Done.

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:259: String _callbacks(var item, List<Function>
callbacks) =>
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> Just because you *can* use a functional style doesn't mean you *must*. :)

I'll concede that Member#hashCode and #== were a bit overboard, but this is a
pretty simple one-liner. I think it reads fine as-is.

> Also, this name doesn't really illuminate. Maybe _runDocumenters?

Done.

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:653: docCode(SourceSpan span, String extraDocs, [bool
showCode = false]) {
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> "extraMarkdown"
> 
> We need to make it clear that this happens before md->html.

Done.

http://codereview.chromium.org/8771054/diff/1/utils/dartdoc/dartdoc.dart#newc...
utils/dartdoc/dartdoc.dart:782: registerTypeCallback(DocTypeFunction fn) =>
_docTypeFns.add(fn);
On 2011/12/06 23:11:23, Bob Nystrom wrote:
> I'd name this "add" instead of "register" to make it clear that it doesn't
> replace a previously added one. Also, it isn't really clear what this callback
> does for the type.
> 
> Maybe addTypeDocumenter(TypeDocumenter documenter)?

Done.

[nweiz, 2011-12-07 19:27:43]

Looping in Siggi to get this reviewed and committed in time for the hackathon.

[Siggi Cherem (dart-lang), 2011-12-07 21:16:12]

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
File client/html/scripts/html_diff.dart (right):

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:155: List<Type> htmlToDomTypes(Type htmlType)
{
some high-level dartdoc on some of this functions would be useful

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:267: } else {
might be useful to check that you don't get a statement that you didn't expect
here. I presume the htmlImpl code doesn't have loops, for instance.

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:306: return
_domMembersFromExpression(expr.self.arguments[0].value, domTypes);
80 col

http://codereview.chromium.org/8771054/diff/9002/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/member.dart#newcode202
frog/member.dart:202: final nameCode = isConstructor ?
constructorName.hashCode() : name.hashCode();
80

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart#newcode387
frog/type.dart:387: bool operator ==(other) =>
I thought that frog maintained that there was a single instance per type (so
this would only be true if === is true)

http://codereview.chromium.org/8771054/diff/9002/utils/dartdoc/dartdoc.dart
File utils/dartdoc/dartdoc.dart (right):

http://codereview.chromium.org/8771054/diff/9002/utils/dartdoc/dartdoc.dart#n...
utils/dartdoc/dartdoc.dart:421: write('Implements ${interfaceStr}. ');
nice (I was very happy when I saw the 'and' is actually included by
joinWithCommas)

[Jacob, 2011-12-07 21:29:12]

lgtm

[nweiz, 2011-12-07 22:09:10]

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
File client/html/scripts/html_diff.dart (right):

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:155: List<Type> htmlToDomTypes(Type htmlType)
{
On 2011/12/07 21:16:13, sigmund wrote:
> some high-level dartdoc on some of this functions would be useful

Added a TODO for this.

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:267: } else {
On 2011/12/07 21:16:13, sigmund wrote:
> might be useful to check that you don't get a statement that you didn't expect
> here. I presume the htmlImpl code doesn't have loops, for instance.

The dumb fallback is intentional. The goal of this code is to parse simple
methods that call the DOM code directly. If the code isn't simple enough, we
don't want any sort of error; we just give up on parsing and assume the author
will add a @domName annotation if it does correspond to something in the DOM.

http://codereview.chromium.org/8771054/diff/8004/client/html/scripts/html_dif...
client/html/scripts/html_diff.dart:306: return
_domMembersFromExpression(expr.self.arguments[0].value, domTypes);
On 2011/12/07 21:16:13, sigmund wrote:
> 80 col

Done.

http://codereview.chromium.org/8771054/diff/9002/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/member.dart#newcode202
frog/member.dart:202: final nameCode = isConstructor ?
constructorName.hashCode() : name.hashCode();
On 2011/12/07 21:16:13, sigmund wrote:
> 80

Done.

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart#newcode387
frog/type.dart:387: bool operator ==(other) =>
On 2011/12/07 21:16:13, sigmund wrote:
> I thought that frog maintained that there was a single instance per type (so
> this would only be true if === is true)

That's true within a single run of the compiler, but we need to compare and hash
types between multiple runs (one for dartdoc and one for html_diff).

[Siggi Cherem (dart-lang), 2011-12-07 22:22:49]

lgtm

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart#newcode387
frog/type.dart:387: bool operator ==(other) =>
On 2011/12/07 22:09:11, nweiz wrote:
> On 2011/12/07 21:16:13, sigmund wrote:
> > I thought that frog maintained that there was a single instance per type (so
> > this would only be true if === is true)
> 
> That's true within a single run of the compiler, but we need to compare and
hash
> types between multiple runs (one for dartdoc and one for html_diff).

I see - I wonder if htmldiff could be done lazily (at the time you get the calls
to addMemberDoc) so that you only do one run of the compiler.

[nweiz, 2011-12-07 22:26:41]

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart#newcode387
frog/type.dart:387: bool operator ==(other) =>
On 2011/12/07 22:22:52, sigmund wrote:
> On 2011/12/07 22:09:11, nweiz wrote:
> > On 2011/12/07 21:16:13, sigmund wrote:
> > > I thought that frog maintained that there was a single instance per type
(so
> > > this would only be true if === is true)
> > 
> > That's true within a single run of the compiler, but we need to compare and
> hash
> > types between multiple runs (one for dartdoc and one for html_diff).
> 
> I see - I wonder if htmldiff could be done lazily (at the time you get the
calls
> to addMemberDoc) so that you only do one run of the compiler.

We need to crawl the entire dart:htmlimpl library before generating docs for any
dart:dom code, since any dart:htmlimpl member could potentially be annotated to
correspond to the dart:dom member we're documenting.

[Siggi Cherem (dart-lang), 2011-12-07 22:32:19]

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8771054/diff/9002/frog/type.dart#newcode387
frog/type.dart:387: bool operator ==(other) =>
On 2011/12/07 22:26:42, nweiz wrote:
> On 2011/12/07 22:22:52, sigmund wrote:
> > On 2011/12/07 22:09:11, nweiz wrote:
> > > On 2011/12/07 21:16:13, sigmund wrote:
> > > > I thought that frog maintained that there was a single instance per type
> (so
> > > > this would only be true if === is true)
> > > 
> > > That's true within a single run of the compiler, but we need to compare
and
> > hash
> > > types between multiple runs (one for dartdoc and one for html_diff).
> > 
> > I see - I wonder if htmldiff could be done lazily (at the time you get the
> calls
> > to addMemberDoc) so that you only do one run of the compiler.
> 
> We need to crawl the entire dart:htmlimpl library before generating docs for
any
> dart:dom code, since any dart:htmlimpl member could potentially be annotated
to
> correspond to the dart:dom member we're documenting.

got it, thx for the clarification.

[Bob Nystrom, 2011-12-07 22:54:29]

LGTM

http://codereview.chromium.org/8771054/diff/15001/client/html/scripts/html_di...
File client/html/scripts/html_diff.dart (right):

http://codereview.chromium.org/8771054/diff/15001/client/html/scripts/html_di...
client/html/scripts/html_diff.dart:64: HtmlDiff() :
For future reference, I think we usually wrap that:
HtmlDiff()
  : domToHtml...

Also, for new collections, I usually use literal form:

domToHtml = <Member, Set<Member>>{}

[nweiz, 2011-12-07 23:01:11]

http://codereview.chromium.org/8771054/diff/15001/client/html/scripts/html_di...
File client/html/scripts/html_diff.dart (right):

http://codereview.chromium.org/8771054/diff/15001/client/html/scripts/html_di...
client/html/scripts/html_diff.dart:64: HtmlDiff() :
On 2011/12/07 22:54:30, Bob Nystrom wrote:
> For future reference, I think we usually wrap that:
> HtmlDiff()
>   : domToHtml...

Noted.

> Also, for new collections, I usually use literal form:
> 
> domToHtml = <Member, Set<Member>>{}

Currently, "<Member, Set<Member>>{}" produces a warning. Apparently the spec
wants all map literals to have string keys.