Integrate MDN content into API documentation.

utils/apidoc/mdn/postProcess.dart

+#library("dump");

[nweiz, 2012/02/01 00:10:39]

Should probably have the same name as the file.

[Jacob, 2012/02/01 07:48:26]

Done.

[Jacob, 2012/02/02 05:26:38]

Added a class level comment to clarify that this file needs to split up into a
quick and dirty pretty printer and a post process step.
// TODO(jacobr): this file conflates pretty printing of the JSON database with
// filtering the database to select the best matches per file.
// Separate out the two tasks as quick and dirty coding is correct for pretty
// printing but more carefully documented code is required for the code
// filtering the database.

+
+#import("../../../frog/lib/node/node.dart");
+#import('../../../frog/lib/node/fs.dart');
+#import("dart:json");
+
+Map database;
+Map allProps;
+Set matchedTypes;

[nweiz, 2012/02/01 00:10:39]

These should have generic parameters.

[Jacob, 2012/02/01 07:48:26]

I don't find that helpful when dealing with JSON.

[nweiz, 2012/02/01 21:23:02]

I find it helpful as a reader of the code. I want to know things like how
deeply-nested the objects are, and once you can get rid of the type-declared
variables you'll need the full declaration to get type inference.

[Jacob, 2012/02/02 05:26:38]

I don't want to write
Map<String, Map<String, Map<String, var>>>
Tried the middle ground of specifying one level more of the type hierarchy. Not
sure if this will work on dartium or trigger type errors. 

[nweiz, 2012/02/02 19:54:34]

A single-level declaration is okay, I suppose, as long as all the variables
assigned to the map values also have single-level generic types declared.

+
+String orEmpty(String str) {
+  return str == null ? "" : str;
+}
+
+/** Returns whether the type has any property matching the specified name. */

[nweiz, 2012/02/01 00:10:39]

s/property/member/

[Jacob, 2012/02/02 05:26:38]

Done.

+bool hasAny(String type, String prop) {
+  Map data = allProps[type];

[nweiz, 2012/02/01 00:10:39]

This type declaration would be unnecessary if allProps had a fully defined type.

There are various other unnecessary types in this file.

[Jacob, 2012/02/02 05:26:38]

Done.

+  return data['properties'].containsKey(prop) ||
+      data['methods'].containsKey(prop) ||
+      data['constants'].containsKey(prop);
+}
+
+List<String> sortStringCollection(Collection<String> collection) {

[nweiz, 2012/02/01 00:10:39]

I'd call this sortCollection and type it as "Collection<Comparable> ->
List<Comparable>". Seems like methods should accept as broad a type as possible,
and the caller should cast to something more specific.

[Jacob, 2012/02/02 05:26:38]

There is little value in being generic when I only need to sort strings.  This
is a helper method in a pretty printer not a generic library call.

[nweiz, 2012/02/02 19:54:34]

There's little value in being String-specific, either. I think making the method
broader when there's no cost to doing so is a good idea, even if you don't
anticipate re-using it. It expresses the intent of the method better, which
makes the code easier to understand. When the method is called
"sortStringCollection", I expect it to do something String-specific and have to
spend extra mental cycles figuring out that it doesn't.

[Jacob, 2012/02/02 22:03:14]

having a sortCollection method would be reasonable. However keep in mind that in
Dart that would mean the output would need to be a 
List<Collection> instead of a List<String> which is a little annoying.
Anyway, if you feel really strongly about this I could go either way.
FWIW, this method could easily become string specific if we decided it was
important to use a case insensitive string comparison method, etc.
Anyway, as a design principle I believe that it is rarely worth it to generalize
code before you have to. If needed it would be trivial to refactor this to a
general method in the future.  Being more specific makes code less abstract so
often easier to follow.  Obviously in this case the code is very simple so it
isn't a big deal one way or the other.

+  List<String> out = new List<String>();
+  out.addAll(collection);
+  out.sort((String a, String b) => a.compareTo(b));
+  return out;
+}
+
+/** Switch from a List of members to a Map of members. */
+Map getMembersMap(Map entry) {

[nweiz, 2012/02/01 00:10:39]

Either change the docstring or make this take a List.

[Jacob, 2012/02/02 05:26:38]

removed comment as it was confusing

+  List rawMembers = entry["members"];
+  Map members = new Map<String, Object>();

[nweiz, 2012/02/01 00:10:39]

s/Object/Dynamic/. Also use a map literal. I guess "<Dynamic>{}" just simplifies
to "{}", so even better.

Also the type declaration for "members" is unnecessary.

[Jacob, 2012/02/02 05:26:38]

Done.

+  for (Map entry in rawMembers) {
+    members[entry['name']] = entry;
+  }
+  return members;
+}
+
+/**
+ * Add all missing members to the string output and return the number of
+ * missing members.
+ */
+int addMissingHelper(StringBuffer sb, String type, Map members,
+                     String propType) {

[nweiz, 2012/02/01 00:10:39]

Helper methods should be declared within the functions that use them as helpers.
This also means you don't have to take sb, type, and members as parameters.

[Jacob, 2012/02/02 05:26:38]

agreed.
done.

+  int total = 0;
+  Map expected = allProps[type][propType];
+  if (expected == null) return total;
+  for(String name in sortStringCollection(expected.getKeys())) {
+    if (!members.containsKey(name)) {
+      total++;
+    sb.add("""

[nweiz, 2012/02/01 00:10:39]

Style nit: incorrect indentation.

[Jacob, 2012/02/02 05:26:38]

this is the correct indentation so that the output HTML is nicely indented.

[nweiz, 2012/02/02 19:54:34]

"<tr>" and "</tr>" should have the same indentation.

[Jacob, 2012/02/02 22:03:14]

fixed

+            <tr class="missing">
+                <td>$name</td>
+                <td></td>
+                <td>Could not find documentation for $propType</td>
+              </tr>
+""");
+    }
+  }
+  return total;
+}
+
+int addMissing(StringBuffer sb, String type, Map members) {
+  int total = 0;
+  total += addMissingHelper(sb, type, members, 'properties');
+  total += addMissingHelper(sb, type, members, 'methods');
+  total += addMissingHelper(sb, type, members, 'constants');
+  return total;
+}
+
+/**
+ * Score an entry using heuristics to determine how well a entry

[nweiz, 2012/02/01 00:10:39]

Grammar nit: "an entry".

[Jacob, 2012/02/02 05:26:38]

Done.

+ * matches the expected list of matching members.
+ * We could be much less naive and penalize spurious methods, prefer entries
+ * with class level comments, etc.

[nweiz, 2012/02/01 00:10:39]

It'd be nice to have an example here of where we get conflicts and what they
look like. Maybe just links to different MDN pages we have to choose between for
some case.

[Jacob, 2012/02/02 05:26:38]

added a big comment

/**
 * Score entries using similarity heuristics calculated from the observed and
 * expected list of members. We could be much less naive and penalize spurious
 * methods, prefer entries with class level comments, etc. This method is
 * needed becase we extract entries for each of the top search results for
 * each class name and rely on these scores to determine which entry was
 * best.  Typically all scores but one will be zero.  Multiple pages have
 * non-zero scores when MDN has multiple pages on the same class or pages on
 * similar classes (e.g. HTMLElement and Element), or pages on Mozilla
 * specific classes that are similar to DOM classes (Console).
 */

[nweiz, 2012/02/02 19:54:34]

This is great.

+ */
+num scoreEntry(Map entry, String type) {

[nweiz, 2012/02/01 00:10:39]

s/num/int/

[Jacob, 2012/02/02 05:26:38]

there isn't any reason why the score won't be a floating point number in the
future.  thus num is correct.

+  num score = 0;
+  if (!entry.containsKey('skipped')) {
+    score++;

[nweiz, 2012/02/01 00:10:39]

Why would we ever want to choose a skipped entry?

[Jacob, 2012/02/02 22:03:14]

I doubt we would. Added a TODO to consider removing skipped entries completely
instead of just giving them lower scores.

+  }
+  if (entry.containsKey("members")) {
+    Map members = getMembersMap(entry);
+    for (String name in members.getKeys()) {

[nweiz, 2012/02/01 00:10:39]

Why are we converting the members list into a map, only to iterate over it as
though it were a list?

[Jacob, 2012/02/02 05:26:38]

to remove any possible duplicates from the original list.

+      if (hasAny(type, name)) {
+        score++;
+      }
+    }
+  }
+  return score;
+}
+
+/**
+ * Given a list of candidates for the documentation for a type, find the one
+ * that is the best.
+ */
+Map pickBestEntry(List entries, String type) {
+  num bestScore = -1;
+  Map bestEntry;
+  for (Map entry in entries) {
+    if (entry != null) {
+    num score = scoreEntry(entry, type);

[nweiz, 2012/02/01 00:10:39]

Indentation.

Alternately, "if (entry == null) continue" on the previous line.

[Jacob, 2012/02/02 05:26:38]

control flow with continue is less readable in general.

[nweiz, 2012/02/02 19:54:34]

It seems to me like it's the same case as "if (...) return". I find it more
readable than deeply-nested conditionals.

[Jacob, 2012/02/02 22:03:14]

Some people go so far as to prohibit 
if (...) return; calls at the top of methods
I think those are fine and can improve readability in a lot of cases.
However, returns in arbitrary locations in the middle of methods are bad and
significantly hurt readability by making the code flow more surprising.
As code is written once and often read multiple times I prefer less surprising
code to more terse code for cases when the extra code bloat is small.

+    if (score > bestScore) {
+      bestScore = score;
+      bestEntry = entry;
+    }
+  }
+  }
+  return bestEntry;
+}
+
+void main() {
+  // Database of code documentation.
+  database = JSON.parse(fs.readFileSync('output/database.json', 'utf8'));
+  // Database of expected property names for each type in WebKit.
+  allProps = JSON.parse(fs.readFileSync('data/dartIdl.json', 'utf8'));
+  // Types we have documentation for.
+  matchedTypes = new Set<String>();
+  int numMissingMethods = 0;
+  int numFoundMethods = 0;
+  int numExtraMethods = 0;
+  int numGen = 0;
+  int numSkipped = 0;
+  final sbSkipped = new StringBuffer();
+  final sbAllExamples = new StringBuffer();
+  final filteredDb = {};
+
+  // Table rows for all obsolete members.
+  final sbObsolete = new StringBuffer();
+  // Main documentation file.
+  final sb = new StringBuffer();

[nweiz, 2012/02/01 00:10:39]

The vast number of string buffers being used in this method make my head spin.
In lieu of an actual template system, I think it would be cleaner to write this
code as though one existed. That is, have an individual method for each file
that will be created; put all string buffer management into those methods. Then
main's responsibility becomes simply packaging the data up into the structures
that each of these faux-template methods needs.

[Jacob, 2012/02/02 05:26:38]

I agree that this code is ugly due to the lack of a template system.  Added a
TODO.  However, it isn't worthwhile to cleanup the style for this debug output
generation code here as it is throw away debugging code.  The only worthwhile
code that needs to be maintainable in this file is the post processing code.  As
mentioned in the TODO at the top of the file I should split that out from this
debugging code.  The post processing code should be maintainable, the debugging
output code should be just quick and dirty and so no extra work should be
performed to make it production quality.

[nweiz, 2012/02/02 19:54:34]

When will the debugging code be removed?

[Jacob, 2012/02/02 22:03:14]

in my copious free time.  If you think it is a priority you are welcome to split
out the actual logic from the code that generates debugging html.

+
+  sb.add("""

[nweiz, 2012/02/01 00:10:39]

Maybe add a TODO here for using a template system once we have one.

[Jacob, 2012/02/02 05:26:38]

Done.

+<html>
+  <head>
+    <style type="text/css">
+      body {
+        background-color: #eee;
+        margin: 10px;
+        font: 14px/1.428 "Lucida Grande", "Lucida Sans Unicode", Lucida, Arial, Helvetica, sans-serif;

[nweiz, 2012/02/01 00:10:39]

Line length.

[Jacob, 2012/02/02 05:26:38]

Done.

+      }
+
+      .debug {
+        color: #888;
+      }
+
+      .compatibility, .links, .see-also, .summary, .members, .example {
+        border: 1px solid #CCC;
+        margin: 5px;
+        padding: 5px;
+      }
+
+      .type, #dart_summary {
+        border: 1px solid;
+        margin-top: 10px;
+        margin-bottom: 10px;
+        padding: 10px;
+        overflow: hidden;
+        background-color: white;
+        -moz-box-shadow: 5px 5px 5px #888;
+        -webkit-box-shadow: 5px 5px 5px #888;
+        box-shadow: 5px 5px 5px #888;
+      }
+
+      #dart_summary {
+        border: 2px solid #00F;
+        margin: 5px;
+        padding: 5px;
+      }
+
+      th {
+        background-color:#ccc;
+        font-weight: bold;
+      }
+
+      tr:nth-child(odd) {
+        background-color:#eee;
+      }
+      tr:nth-child(even) {
+              background-color:#fff;
+            }
+
+      tr:nth-child(odd).unknown {
+        background-color:#dd0;
+      }
+      tr:nth-child(even).unknown {
+              background-color:#ff0;
+            }
+
+      tr:nth-child(odd).missing {
+        background-color:#d88;
+      }
+      tr:nth-child(even).missing {
+              background-color:#faa;
+            }
+
+            li.unknown {
+        color: #f00;
+            }
+
+            td, th {
+                vertical-align: top;
+            }
+    </style>
+    <title>Doc Dump</title>
+  </head>
+  <body>
+    <h1>Doc Dump</h1>
+    <ul>
+      <li><a href="#dart_summary">Summary</a></li>
+    </li>
+""");
+
+  for (String type in sortStringCollection(database.getKeys())) {
+    Map entry = pickBestEntry(database[type], type);
+    filteredDb[type] = entry;
+    if (entry == null || entry.containsKey('skipped')) {
+      numSkipped++;
+      sbSkipped.add("""
+    <li id="$type">
+      <a target="_blank" href="http://www.google.com/cse?cx=017193972565947830266%3Awpqsk6dy6ee&ie=UTF-8&q=$type">
+        $type
+      </a> -- Title: ${entry == null ? "???" : entry["title"]} -- Issue: ${entry == null ? "???" : entry['cause']}
+      -- <a target="_blank" href="${entry == null ? "???" : entry["srcUrl"]}">scraped url</a>

[nweiz, 2012/02/01 00:10:39]

Line lengths.

[Jacob, 2012/02/02 05:26:38]

Done.

+    </li>""");
+      continue;
+    }
+    matchedTypes.add(type);
+    numGen++;
+    StringBuffer sbSections = new StringBuffer();
+    StringBuffer sbMembers = new StringBuffer();
+    StringBuffer sbExamples = new StringBuffer();
+    if (entry.containsKey("members")) {
+        Map members = getMembersMap(entry);

[nweiz, 2012/02/01 00:10:39]

Indentation, here and many other places below

[Jacob, 2012/02/02 05:26:38]

Done.

+        sbMembers.add("""
+            <div class="members">
+          <h3><span class="debug">[dart]</span> Members</h3>
+          <table>
+            <tbody>
+              <tr>
+                <th>Name</th><th>Description</th><th>IDL</th><th>Status</th>
+              </tr>
+""");
+      for (String name in sortStringCollection(members.getKeys())) {
+          Map memberData = members[name];
+          bool unknown = !hasAny(type, name);
+          StringBuffer classes = new StringBuffer();
+        if (unknown) classes.add("unknown ");
+        if (unknown) {
+          numExtraMethods++;
+        } else {
+          numFoundMethods++;
+        }
+
+        final sbMember = new StringBuffer();
+
+      if (memberData.containsKey('url')) {
+        sbMember.add("""
+                         <td><a href="${memberData['url']}">$name</a></td>
+""");
+      } else {
+        sbMember.add("""
+                         <td>$name</td>
+""");
+    }
+        sbMember.add("""
+                             <td>${memberData['help']}</td>
+             <td>
+               <pre>${orEmpty(memberData['idl'])}</pre>
+             </td>
+             <td>${memberData['obsolete'] == true ? "Obsolete" : ""}</td>
+""");
+        if (memberData['obsolete'] == true) {
+          sbObsolete.add("<tr class='$classes'><td>$type</td>$sbMember</tr>");
+        }
+        sbMembers.add("<tr class='$classes'>$sbMember</tr>");
+        }
+
+      numMissingMethods += addMissing(sbMembers, type, members);
+
+      sbMembers.add("""
+            </tbody>
+          </table>
+        </div>
+""");
+    }
+    for (String sectionName in
+        ["summary", "constructor", "compatibility", "specification", "seeAlso"])

[nweiz, 2012/02/01 00:10:39]

Where's the opening { for this loop?

[Jacob, 2012/02/02 05:26:38]

wow... i wish that was a compile warning or at least a lint warning.

+    if (entry.containsKey(sectionName)) {
+      sbSections.add("""
+      <div class="$sectionName">
+        <h3><span class="debug">[Dart]</span> $sectionName</h3>
+        ${entry[sectionName]}
+      </div>
+""");
+    }
+    if (entry.containsKey("links")) {
+      sbSections.add("""
+      <div class="links">
+        <h3><span class="debug">[Dart]</span> Specification</h3>
+        <ul>
+""");
+        List links = entry["links"];
+        for (Map link in links) {
+          sbSections.add("""
+      <li><a href="${link['href']}">${link['title']}</a></li>
+""");
+        }
+      sbSections.add("""
+        </ul>
+      </div>
+""");
+      }
+    if (entry.containsKey("examples")) {
+        for (String example in entry["examples"]) {
+          sbExamples.add("""
+            <div class="example">
+                  <h3><span class="debug">[Dart]</span> Example</h3>
+                  $example
+                </div>
+""");
+      }
+    }
+
+    String title = entry['title'];
+    if (title != type) {
+        title = '<h4>Dart type: $type</h4><h2>${title}</h2>';

[nweiz, 2012/02/01 00:10:39]

Style nit: $title, here and line 330.

[Jacob, 2012/02/02 05:26:38]

Done.

+    } else {
+        title = '<h2>${title}</h2>';
+    }
+    sb.add("""
+    <div class='type' id="$type">
+      <a href='${entry['srcUrl']}'>$title</a>
+$sbSections
+$sbExamples
+$sbMembers
+    </div>
+""");
+    if (sbExamples.length > 0) {
+      sbAllExamples.add("""
+    <div class='type' id="$type">
+      <a href='${entry['srcUrl']}'>$title</a>
+      ${sbExamples.toString()}

[nweiz, 2012/02/01 00:10:39]

Style nit: $sbExamples

[Jacob, 2012/02/02 05:26:38]

Done.

+    </div>
+""");
+    }
+  }
+
+  for (String type in sortStringCollection(allProps.getKeys())) {
+    if (!matchedTypes.contains(type) &&
+        !database.containsKey(type)) {
+      numSkipped++;
+      sbSkipped.add("""
+    <li class="unknown" id="$type">
+      <a target="_blank" href="http://www.google.com/cse?cx=017193972565947830266%3Awpqsk6dy6ee&ie=UTF-8&q=$type">
+        $type
+      </a>
+    </li>
+""");
+    }
+  }
+
+  sb.add("""
+<div id="#dart_summary">
+  <h2>Summary</h2>
+  <h3>Generated docs for ${numGen} classes out of a possible ${allProps.getKeys().length}</h3>
+  <h3>Found documentation for $numFoundMethods methods listed in WebKit</h3>
+  <h3>Found documentation for $numExtraMethods methods not listed in WebKit</h3>
+  <h3>Unable to find documentation for $numMissingMethods methods not present in WebKit</h3>
+  <h3>Skipped generating documentation for $numSkipped classes due to no plausible matching files</h3>

[nweiz, 2012/02/01 00:10:39]

Line lengths

[Jacob, 2012/02/02 05:26:38]

Done.

+  <ul>
+$sbSkipped
+  </ul>
+</div>
+""");
+  sb.add("""
+  </body>
+</html>
+""");
+
+  fs.writeFileSync("output/database.html", sb.toString());
+
+  fs.writeFileSync("output/examples.html", """
+<html>
+  <head>
+    <style type="text/css">
+      body {
+        background-color: #eee;
+        margin: 10px;
+        font: 14px/1.428 "Lucida Grande", "Lucida Sans Unicode", Lucida, Arial, Helvetica, sans-serif;

[nweiz, 2012/02/01 00:10:39]

Line length

[Jacob, 2012/02/02 05:26:38]

Done.

+      }
+
+      .debug {
+        color: #888;
+      }
+
+      .example {
+        border: 1px solid #CCC;
+        margin: 5px;
+        padding: 5px;
+      }
+
+      .type {
+        border: 1px solid;
+        margin-top: 10px;
+        margin-bottom: 10px;
+        padding: 10px;
+        overflow: hidden;
+        background-color: white;
+        -moz-box-shadow: 5px 5px 5px #888;
+        -webkit-box-shadow: 5px 5px 5px #888;
+        box-shadow: 5px 5px 5px #888;
+      }
+    </style>
+    <title>All examples</title>
+  </head>
+  <body>
+    <h1>All examples</h1>
+$sbAllExamples
+  </body>
+ </html>
+""");
+
+  fs.writeFileSync("output/obsolete.html", """
+<html>
+  <head>
+    <style type="text/css">
+      body {
+        background-color: #eee;
+        margin: 10px;
+        font: 14px/1.428 "Lucida Grande", "Lucida Sans Unicode", Lucida, Arial, Helvetica, sans-serif;

[nweiz, 2012/02/01 00:10:39]

Line length

[Jacob, 2012/02/02 05:26:38]

Done.

+      }
+
+      .debug {
+        color: #888;
+      }
+
+      .type {
+        border: 1px solid;
+        margin-top: 10px;
+        margin-bottom: 10px;
+        padding: 10px;
+        overflow: hidden;
+        background-color: white;
+        -moz-box-shadow: 5px 5px 5px #888;
+        -webkit-box-shadow: 5px 5px 5px #888;
+        box-shadow: 5px 5px 5px #888;
+      }
+    </style>
+    <title>Methods marked as obsolete</title>
+  </head>
+  <body>
+    <h1>Methods marked as obsolete</h1>
+          <table>

[nweiz, 2012/02/01 00:10:39]

Indentation

[Jacob, 2012/02/02 05:26:38]

Done.

+            <tbody>
+              <tr>
+                <th>Type</th><th>Name</th><th>Description</th><th>IDL</th><th>Status</th>

[nweiz, 2012/02/01 00:10:39]

Line length

[Jacob, 2012/02/02 05:26:38]

Done.

+              </tr>
+$sbObsolete
+    </tbody>
+   </table>
+  </body>
+ </html>
+ """);
+
+  fs.writeFileSync("output/database.filtered.json", JSON.stringify(filteredDb));
+}