LGTM with nits. No need to rereview if you like the suggested changes.
These changes will make SOYC a lot easier to understand. http://gwt-code-reviews.appspot.com/566801/diff/1/3 File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/566801/diff/1/3#newcode179 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:179: outFile.println("if (element.name == \"packageBreakdown\") {"); Putting ifs here doesn't scale well if we add more popups -- and we should! There are multiple ways to avoid an ever increasing chain of ifs here. A simple way would be to remove the "element" parameter and add one with the ID of the help popup. http://gwt-code-reviews.appspot.com/566801/diff/1/3#newcode284 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:284: "onMouseOut=\"hide(this);\">Package breakdown</a></h2></div>"); If you go with the changed parameter, then show(this) would be changed to show(\"packageBreakdownPopup\");, and likewise for hide(). http://gwt-code-reviews.appspot.com/566801/diff/1/3#newcode359 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:359: outFile.println("<th>Size <span class=\"soyc-th-units\">(Bytes)</span></th>"); Nice. It pains me to think how many times I looked at that table header and didn't notice the problem. http://gwt-code-reviews.appspot.com/566801/diff/1/3#newcode728 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:728: TreeMap<Float, Set<String> > sortedCodeTypes = new TreeMap<Float, Set<String> >( Very nice, and likewise for the other ones. Could you also change the keys from Float to Integer? Comparison on floats is begging for weird corner cases. http://gwt-code-reviews.appspot.com/566801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
