LGTM The actual patch looks good to me (see only two small comments below), but I am a little concerned about the unescaping of XML. Please see my comment below.
http://gwt-code-reviews.appspot.com/33843/diff/1/6 File tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/33843/diff/1/6#newcode629 Line 629: if (i == numSplitPoints + 1 && numSplitPoints == 0) { Minor pick: could say if (i == 1 && numSplitPoints == 0) here. I suspect you did it the more elaborate way to make it easier to follow, though. http://gwt-code-reviews.appspot.com/33843/diff/1/2 File tools/soyc-vis/src/com/google/gwt/soyc/Settings.java (right): http://gwt-code-reviews.appspot.com/33843/diff/1/2#newcode120 Line 120: throw new ArgumentListException("The -resources option is required"); Don't you want to show settingsHelp() here anyway? http://gwt-code-reviews.appspot.com/33843/diff/1/3 File tools/soyc-vis/src/com/google/gwt/soyc/SoycDashboard.java (left): http://gwt-code-reviews.appspot.com/33843/diff/1/3#oldcode174 Line 174: return unescaped; I'm unsure when we stopped calling this method, but did we make sure our byte counts (when compared to the size of the file on disk, non-gzipped) are correct? http://gwt-code-reviews.appspot.com/33843 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
