Fix ApiDoc to work with new dart:html code. Tweak dart:html generator to generate clearer @domName …

Fix ApiDoc to work with new dart:html code. Tweak dart:html generator to generate clearer @domName annotations.


BUG=
TEST=

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

[Jacob, 2012-04-12 17:38:03]

nweiz please review html_diff.dart
rnystrom please review dartdoc and apidoc
sra please review generator tweaks

The generator output is unchanged except for the longer comments.

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartdo...
File lib/dom/scripts/dartdomgenerator.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartdo...
lib/dom/scripts/dartdomgenerator.py:107: generator.RenameTypes(webkit_database,
_webkit_renames, True)
This parameter was inverted previously. Cleaning up.

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/system...
File lib/dom/scripts/systemhtml.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:647: self._type_comment_emitter.Emit("/// @domName
$DOMNAME",
Needed so that api_doc knows what the source regular DOM class is for each html
class.

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/system...
lib/dom/scripts/systemhtml.py:696: self._members_emitter.Emit('\n  /** @domName
$DOMINTERFACE.$DOMNAME */',
Always specify the interface name as well as the method name as some classes can
have methods from multiple sources.

[Bob Nystrom, 2012-04-12 17:54:06]

Dart code changes LGTM, assuming you ran apidoc locally and everything checked
out OK. Thanks for doing this!

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dartdoc/utils.dart
File lib/dartdoc/utils.dart (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dartdoc/utils.dart...
lib/dartdoc/utils.dart:53: // Hack: make sure $dom methods show up last in the
list not first.
Take credit for it. :)

// TODO(jacobr): ...

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
File lib/dom/scripts/dartgenerator.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
lib/dom/scripts/dartgenerator.py:113: for member in interface.operations +
interface.constants + interface.attributes:
long line

[sra1, 2012-04-12 17:58:21]

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
File lib/dom/scripts/dartgenerator.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
lib/dom/scripts/dartgenerator.py:113: for member in interface.operations +
interface.constants + interface.attributes:
line length.
for member in (....
    ...):
should do the trick

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/idlnod...
File lib/dom/scripts/idlnode.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/idlnod...
lib/dom/scripts/idlnode.py:371: self.javascript_binding_name = self.id
Since this field is about documentation and not the targeted JS code, the name
should contain 'doc', e.g.
doc_js_name and doc_js_interface_name

[nweiz, 2012-04-12 19:40:51]

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (left):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:257: if (htmlMember.name == 'get:on') {
Why don't we need special logic for events anymore?

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:130: if (tags.containsKey('domName')) {
if (!tags.containsKey('domName')) return <Type>[];

[sra1, 2012-04-13 01:02:35]

I'm lgtm provided we keep the doc gen and code gen concerns separate, i.e.
with fields doc_js_interface_name and doc_js_member_name.

javascript_binding_name is used in native "*Foo" and if we make that a set, we
will want the separation.

[Jacob, 2012-04-13 08:56:01]

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
File lib/dom/scripts/dartgenerator.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
lib/dom/scripts/dartgenerator.py:113: for member in interface.operations +
interface.constants + interface.attributes:
On 2012/04/12 17:58:21, sra1 wrote:
> line length.
> for member in (....
>     ...):
> should do the trick

Done.

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/dartge...
lib/dom/scripts/dartgenerator.py:113: for member in interface.operations +
interface.constants + interface.attributes:
On 2012/04/12 17:54:06, Bob Nystrom wrote:
> long line

Done.

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/idlnod...
File lib/dom/scripts/idlnode.py (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/lib/dom/scripts/idlnod...
lib/dom/scripts/idlnode.py:371: self.javascript_binding_name = self.id
On 2012/04/12 17:58:21, sra1 wrote:
> Since this field is about documentation and not the targeted JS code, the name
> should contain 'doc', e.g.
> doc_js_name and doc_js_interface_name

Done.

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (left):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:257: if (htmlMember.name == 'get:on') {
On 2012/04/12 19:40:51, nweiz wrote:
> Why don't we need special logic for events anymore?

The dart:html generator adds these annotations so this doesn't need to be hard
coded here.

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:130: if (tags.containsKey('domName')) {
On 2012/04/12 19:40:51, nweiz wrote:
> if (!tags.containsKey('domName')) return <Type>[];

not sure that is an improvement

[nweiz, 2012-04-13 18:48:09]

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:130: if (tags.containsKey('domName')) {
On 2012/04/13 08:56:01, Jacob wrote:
> On 2012/04/12 19:40:51, nweiz wrote:
> > if (!tags.containsKey('domName')) return <Type>[];
> 
> not sure that is an improvement

I think it's usually clearer to reduce the nesting in a method where possible.
It means the reader is responsible for holding fewer code paths in their head at
once.

[Jacob, 2012-04-16 07:44:39]

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
File utils/apidoc/html_diff.dart (right):

https://chromiumcodereview.appspot.com/10068019/diff/1/utils/apidoc/html_diff...
utils/apidoc/html_diff.dart:130: if (tags.containsKey('domName')) {
On 2012/04/13 18:48:09, nweiz wrote:
> On 2012/04/13 08:56:01, Jacob wrote:
> > On 2012/04/12 19:40:51, nweiz wrote:
> > > if (!tags.containsKey('domName')) return <Type>[];
> > 
> > not sure that is an improvement
> 
> I think it's usually clearer to reduce the nesting in a method where possible.
> It means the reader is responsible for holding fewer code paths in their head
at
> 

In general I agree that lower nesting levels are better. However, I also believe
simple conditionals are better than more complex conditionals and having fewer
returns nested in the middle of a function is better.

[Bob Nystrom, 2012-04-16 16:39:29]

On 2012/04/16 07:44:39, Jacob wrote:
> In general I agree that lower nesting levels are better. However, I also
believe
> simple conditionals are better than more complex conditionals and having fewer
> returns nested in the middle of a function is better.

Given a language that has try/catch/finally and garbage collection (which Dart
has), I personally prefer early returns over nesting. I think early returns are
problematic in languages where resource management can be tricky, but I don't
think we have that problem in Dart. Given that, I prefer the simpler flow
control that early returns give you.

[Jacob, 2012-04-16 16:42:09]

On 2012/04/16 16:39:29, Bob Nystrom wrote:
> On 2012/04/16 07:44:39, Jacob wrote:
> > In general I agree that lower nesting levels are better. However, I also
> believe
> > simple conditionals are better than more complex conditionals and having
fewer
> > returns nested in the middle of a function is better.
> 
> Given a language that has try/catch/finally and garbage collection (which Dart
> has), I personally prefer early returns over nesting. I think early returns
are
> problematic in languages where resource management can be tricky, but I don't
> think we have that problem in Dart. Given that, I prefer the simpler flow
> control that early returns give you.

A bunch of early returns right at the top of a function are fine.  What I
dislike is early returns strewn throughout a function.  They give the illusion
of simpler control flow when the control flow is no simpler than if there were
nested blocks.

[nweiz, 2012-04-16 17:58:15]

lgtm

On 2012/04/16 16:42:09, Jacob wrote:
> On 2012/04/16 16:39:29, Bob Nystrom wrote:
> > On 2012/04/16 07:44:39, Jacob wrote:
> > > In general I agree that lower nesting levels are better. However, I also
> > believe
> > > simple conditionals are better than more complex conditionals and having
> fewer
> > > returns nested in the middle of a function is better.
> > 
> > Given a language that has try/catch/finally and garbage collection (which
Dart
> > has), I personally prefer early returns over nesting. I think early returns
> are
> > problematic in languages where resource management can be tricky, but I
don't
> > think we have that problem in Dart. Given that, I prefer the simpler flow
> > control that early returns give you.
> 
> A bunch of early returns right at the top of a function are fine.  What I
> dislike is early returns strewn throughout a function.  They give the illusion
> of simpler control flow when the control flow is no simpler than if there were
> nested blocks.

I disagree, but I'm not going to press on the issue any harder.