[gwt-contrib] Re: Reduces size of the compile report (soyc) for large projects with many (issue1171801)
LGTM, but please not my comments...
SOYC is becoming smart!! One thing I'm wondering is whether we have a
good way of making sure we still get the right information everywhere
(the smarter it gets, the harder it gets to follow in some sense, e.g.,
one-character function names, etc.). There's SoycTest, but when I wrote
it, it was pretty empty (just checking for the existence of a file or
two). Do you think medium term it would be worth it to add some tests
to that to make sure we're getting the right entries in the different
categories?
http://gwt-code-reviews.appspot.com/1171801/diff/1/3
File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(right):
http://gwt-code-reviews.appspot.com/1171801/diff/1/3#newcode115
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:115:
public void freeze() {
I've been trying to come up with a way in which you would not have to
call freeze() and still be safe. I don't have a good solution -- my
only gripe here is that if you forget to call freeze(), you won't get
any values. Seems like the only way to get around this is to call
freeze whenever something is put in the builder, but that seems very
overheady. The only other thing I can come up with is for frozen and
builder to actually be the same thing (so do an in-place reordering when
you call freeze -- also see my comment below). In that case, if you
forget to call freeze, you still have access to the data, just not in an
optimal way. Do you have any other brilliant ideas?
http://gwt-code-reviews.appspot.com/1171801/diff/1/3#newcode126
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:126: }
What happens if your count has more than 8 digits?
http://gwt-code-reviews.appspot.com/1171801/diff/1/3#newcode131
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:131:
builder = null;
So the only thing I'm wondering is why you're not just using a
TreeMapString, Integer (for frozen) and sorting it by value rather
than key (in the comparator that you give it). Would that not get you
around having to create the valus TreeMap?
http://gwt-code-reviews.appspot.com/1171801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Reduces the size of the soyc report by combining many s (issue1139801)
LGTM. This is another great change - thanks! http://gwt-code-reviews.appspot.com/1139801/diff/1/2 File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/1139801/diff/1/2#newcode789 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:789: outFile.println(a name=\ + filename(className) + \/ah3 + className + /h3); Longer than 100 chars? http://gwt-code-reviews.appspot.com/1139801/diff/1/2#newcode1445 dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:1445: String drillDownFileName = classesInPackageFileName(breakdown, getPermutationId()) + # + filename(packageName); 100 chars? http://gwt-code-reviews.appspot.com/1139801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes part of the Compiler Report (SOYC) smaller by (issue1123801)
LGTM. Nice! I would probably just add a test for empty strings. http://gwt-code-reviews.appspot.com/1123801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes part of the Compiler Report (SOYC) smaller by (issue1123801)
http://gwt-code-reviews.appspot.com/1123801/diff/1/2
File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(right):
http://gwt-code-reviews.appspot.com/1123801/diff/1/2#newcode879
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:879:
return fullMethodName.substring(index + 2);
I think this will crash when you pass an empty string, a string of size
1, or any string that ends with ::.
http://gwt-code-reviews.appspot.com/1123801/diff/1/2#newcode903
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:903:
return packageAndClass.substring(0, endIndex);
Nit: You could get away without having to do two .substring calls (as
they're expensive).
return packageAndClass.substring(0, packageAndClass.lastIndexOf('.'));
instead of the last 3 lines.
(Actually, can there ever be a . after a ::? If not, then you can just
do
return packageAndClass.substring(0, packageAndClass.lastIndexOf('.'));
for the whole method).
http://gwt-code-reviews.appspot.com/1123801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes part of the Compiler Report (SOYC) smaller by (issue1123801)
Really sorry to be so picky, but if you call getClassSubstring(String
fullMethodName) with something like myClass, it'll throw an
IndexOutOfBoundsException (start longer than end). It'll also fail for
the empty string (end: 0, start: 1).
BTW, I can't see the side-by-side diff, not sure if there's a problem
with your patch set or not.
On 2010/11/19 21:22:47, zundel wrote:
updated patch
http://gwt-code-reviews.appspot.com/1123801/diff/1/2
File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(right):
http://gwt-code-reviews.appspot.com/1123801/diff/1/2#newcode879
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:879:
return
fullMethodName.substring(index + 2);
On 2010/11/19 19:10:43, kathrin wrote:
I think this will crash when you pass an empty string, a string of
size 1, or
any string that ends with ::.
I added a check for the indexOf failing and returned the empty string.
I think if you pass xxx:: it will return the empty string.
http://download.oracle.com/javase/1.4.2/docs/api/java/lang/String.html#substring%28int)
http://gwt-code-reviews.appspot.com/1123801/diff/1/2#newcode903
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java:903:
return
packageAndClass.substring(0, endIndex);
On 2010/11/19 19:10:43, kathrin wrote:
Nit: You could get away without having to do two .substring calls
(as they're
expensive).
return packageAndClass.substring(0,
packageAndClass.lastIndexOf('.')); instead
of the last 3 lines.
(Actually, can there ever be a . after a ::? If not, then you can
just do
return packageAndClass.substring(0,
packageAndClass.lastIndexOf('.')); for the
whole method).
yes, the second approach would be much more efficient.
Done.
http://gwt-code-reviews.appspot.com/1123801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove extra slash from request URL of Showcase source files to be compatible with servers that ... (issue961801)
LGTM http://gwt-code-reviews.appspot.com/961801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Making Showcase generate a hidden site map to Showcase to make it crawlable. Also ensuring that ... (issue952801)
LGTM http://gwt-code-reviews.appspot.com/952801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Converts two new files from dos EOL format to unix EOL format. (issue854801)
Reviewers: kjin, Description: Converts two new files from dos EOL format to unix EOL format. Review by: k...@google.com Please review this at http://gwt-code-reviews.appspot.com/854801/show Affected files: M user/src/com/google/gwt/validation/rebind/GwtSpecificValidatorGenerator.java M user/src/com/google/gwt/validation/rebind/ValidatorGenerator.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Delay start of history token transitions in HistoryTest.testHistory() so that when tokens are lo... (issue850802)
LGTM http://gwt-code-reviews.appspot.com/850802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes a bug in the compile report dashboard where entries with same sizes were (issue566801)
All done - thanks, Lex!
On 2010/05/26 18:19:26, Lex wrote:
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(thSize 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:
TreeMapFloat, SetString sortedCodeTypes = new TreeMapFloat,
SetString (
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
[gwt-contrib] Fixes a bug in the compile report dashboard where entries with same sizes were (issue566801)
Reviewers: Lex, Description: Fixes a bug in the compile report dashboard where entries with same sizes were being swallowed. It also now attributes fields to the corresponding packages, and gives additional hints about what is broken down and what isn't. Please review this at http://gwt-code-reviews.appspot.com/566801/show Affected files: M dev/core/src/com/google/gwt/core/ext/soyc/impl/SizeMapRecorder.java M dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java M dev/core/src/com/google/gwt/soyc/SoycDashboard.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: When SoycReportLinker looks for report files, it now tolerates (issue545801)
LGTM http://gwt-code-reviews.appspot.com/545801/diff/1/3 File dev/core/test/com/google/gwt/core/linker/SoycReportLinkerTest.java (right): http://gwt-code-reviews.appspot.com/545801/diff/1/3#newcode80 dev/core/test/com/google/gwt/core/linker/SoycReportLinkerTest.java:80: } This test is good, but it wouldn't have caught the bug that alerted us to a problem: we had an index.html file but it didn't contain anything beyond the header. Do you think it's worthwhile actually looking inside the produced file? Maybe you could do this in SoycTest, where we already look for actual files produced by Hello. http://gwt-code-reviews.appspot.com/545801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Ensure static initialization is atomic (found by findbugs) (issue341802)
LGTM http://gwt-code-reviews.appspot.com/341802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Changes for crawling:
http://gwt-code-reviews.appspot.com/161801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Changes for crawling:
On 2010/03/11 16:45:34, kathrin wrote: Hi Amit, after a lengthy discussion with Joel, we decided to get rid of the CrawlableHyperlink widget. The issue is that it doesn't add enough useful functionality, because the app writer still needs to handle the ! when actually navigating the app to a history state. For this reason, we will recommend that people do this process manually, which is the same amount of work. I got rid of the widget and made the necessary changes to Showcase - could you have another look? Thanks! kathrin http://gwt-code-reviews.appspot.com/161801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Changes for crawling:
Reviewers: amtimanjhi_google.com, Description: Changes for crawling: - CrawlableHyperlink - client-side changes to Showcase sample Review at http://gwt-code-reviews.appspot.com/161801 Please review this at http://gwt-code-reviews.appspot.com/163802 Affected files: M samples/showcase/src/com/google/gwt/sample/showcase/client/Showcase.java M samples/showcase/war/Showcase.html A user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java A user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Changes for crawling:
http://gwt-code-reviews.appspot.com/161801/diff/1/3 File samples/showcase/war/Showcase.html (right): http://gwt-code-reviews.appspot.com/161801/diff/1/3#newcode4 Line 4: script language='javascript' The reason I add the title programmatically is because it changes to reflect the widget that is being shown off. This makes for more descriptive titles, which is especially important for indexing. [As an aside, I personally feel there should be nothing or close to nothing in the html file. I see your point, but if your browser doesn't run JS, this title isn't going to help you a whole lot, either... But if you feel strongly about it, I'll put it back in, but it will just be overridden if the browser does run JS.] On 2010/03/08 23:29:44, amitmanjhi wrote: Any disadvantage to leaving the title here (even though you are setting it later)? At least, browsers that can't run JS can see the title. http://gwt-code-reviews.appspot.com/161801/diff/1/4 File user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java (right): http://gwt-code-reviews.appspot.com/161801/diff/1/4#newcode58 Line 58: } I'm not sure I understand. Because constructors aren't inherited, I wouldn't be able to do Hyperlink link = new CrawlableHyperlink(Click Me, myToken); if I didn't override them. Do you have a specific suggestion? Great point about the more descriptive javadoc. I hope I've improved it. On 2010/03/08 23:29:44, amitmanjhi wrote: Why override these constructors if the override is not doing anything useful? Shouldn't over-riding just the setTargetHistoryToken method and having a more descriptive Javadoc for this class suffice? http://gwt-code-reviews.appspot.com/161801/diff/1/5 File user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java (right): http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode28 Line 28: } On 2010/03/08 23:29:44, amitmanjhi wrote: change to com.google.gwt.user.DebugTest, as in HyperLinkTest. Done. http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode36 Line 36: Thanks! Done. On 2010/03/08 23:29:44, amitmanjhi wrote: Arguments of assertEquals must be swapped. It is assertEquals(expected, actual). http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode41 Line 41: assertEquals(historyToken, !myToken); On 2010/03/08 23:29:44, amitmanjhi wrote: historyToken can be inlined everywhere. Done. http://gwt-code-reviews.appspot.com/161801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Changes for crawling:
Reviewers: amitmanjhi, Description: Changes for crawling: - CrawlableHyperlink - client-side changes to Showcase sample Review at http://gwt-code-reviews.appspot.com/161801 Please review this at http://gwt-code-reviews.appspot.com/165801 Affected files: M samples/showcase/src/com/google/gwt/sample/showcase/client/Showcase.java M samples/showcase/war/Showcase.html A user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java A user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Changes for crawling:
Reviewers: amitmanjhi, Description: Changes for crawling: - CrawlableHyperlink - client-side changes to Showcase sample Please review this at http://gwt-code-reviews.appspot.com/161801 Affected files: M samples/showcase/src/com/google/gwt/sample/showcase/client/Showcase.java M samples/showcase/war/Showcase.html A user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java A user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: URI-escape cookies (addresses external issue 4365)
All done, thanks. Committed at r7354.
On 2009/12/27 03:29:04, Dan Rice wrote:
LGTM with minor nits.
http://gwt-code-reviews.appspot.com/128801/diff/1/3
File user/src/com/google/gwt/user/client/Cookies.java (right):
http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode112
Line 112: if (uriEncoding) {
Prettier and less duplicated code to do:
if (uriEncoding) {
name = uriEncode(name);
}
removeCookieNative(name);
http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode115
Line 115: else {
} else {
on same line
http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode128
Line 128: if (uriEncoding) {
Ditto
http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode184
Line 184: throw new IllegalArgumentException(Illegal cookie
format.);
Include name and value in exception message, maybe distinguish between
bad name
and bad value to make debugging easier.
http://gwt-code-reviews.appspot.com/128801/diff/1/2
File user/test/com/google/gwt/user/client/CookieTest.java (right):
http://gwt-code-reviews.appspot.com/128801/diff/1/2#newcode166
Line 166: // Make sure cookie values are URI encoded
Probably best to add 'Cookies.setUriEncode(true);' here (redundantly)
to avoid
dependency between sections of this method.
http://gwt-code-reviews.appspot.com/128801
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix two CompileReport XML issues
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? It fixes two XML issues in the CompileReport: 1) There was a bug in the escapeXML method that would let all special characters after the first one slip through. This is now fixed. 2) Because of the difference in encoding for Java and for HTML as displayed in the browser, some characters were not displayed properly. Thanks! kathrin Please review this at http://gwt-code-reviews.appspot.com/126813 Affected files: dev/core/src/com/google/gwt/core/ext/soyc/impl/SizeMapRecorder.java dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Last chance SOYC-Compile Report cleanup
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? I replaced several user-facing mentions of SOYC with Compile Report to achieve more consistency, especially for people that don't yet know what SOYC is. Thanks, kathrin Please review this at http://gwt-code-reviews.appspot.com/112810 Affected files: dev/core/src/com/google/gwt/core/ext/linker/impl/StandardCompilationAnalysis.java dev/core/src/com/google/gwt/core/ext/soyc/impl/DependencyRecorder.java dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java dev/core/src/com/google/gwt/core/ext/soyc/impl/StoryRecorder.java dev/core/src/com/google/gwt/core/linker/SoycReportLinker.java dev/core/src/com/google/gwt/dev/jjs/Correlation.java dev/core/src/com/google/gwt/dev/jjs/CorrelationFactory.java dev/core/src/com/google/gwt/dev/util/arg/OptionSoycDetailed.java dev/core/src/com/google/gwt/soyc/CodeCollection.java dev/core/src/com/google/gwt/soyc/GlobalInformation.java dev/core/src/com/google/gwt/soyc/Settings.java dev/core/src/com/google/gwt/soyc/io/OutputDirectory.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Make compiler log start counting permutations at 0 (again)
Reviewers: scottb,
Description:
Scott,
could you review this simple patch for me? It reverts an earlier
change, so that the compiler log starts counting permutations at 0
again. While less pretty, it's consistent with everything else.
Thanks,
kathrin
Please review this at http://gwt-code-reviews.appspot.com/112803
Affected files:
dev/core/src/com/google/gwt/dev/PermutationWorkerFactory.java
dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
Index: dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
===
--- dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(revision 7071)
+++ dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(working copy)
@@ -216,8 +216,7 @@
PropertyOracle[] propertyOracles = permutation.getPropertyOracles();
int permutationId = permutation.getId();
MapString, String rebindAnswers = permutation.getRebindAnswers();
-int printId = permutationId + 1;
-logger.log(TreeLogger.INFO, Compiling permutation + printId
+ ...);
+logger.log(TreeLogger.INFO, Compiling permutation + permutationId
+ ...);
long permStart = System.currentTimeMillis();
try {
if (JProgram.isTracingEnabled()) {
Index: dev/core/src/com/google/gwt/dev/PermutationWorkerFactory.java
===
--- dev/core/src/com/google/gwt/dev/PermutationWorkerFactory.java
(revision
7071)
+++ dev/core/src/com/google/gwt/dev/PermutationWorkerFactory.java
(working
copy)
@@ -249,9 +249,8 @@
ListWork work = new ArrayListWork(permutations.length);
for (int i = 0; i permutations.length; ++i) {
Permutation perm = permutations[i];
- int printId = perm.getId() + 1;
logger.log(TreeLogger.DEBUG,
- Creating worker permutation + printId + of +
permutations.length);
+ Creating worker permutation + perm.getId() + of +
permutations.length);
work.add(new Work(logger, perm, resultFiles.get(i)));
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make compiler log start counting permutations at 0 (again)
On 2009/11/23 16:40:11, scottb wrote: LVGTM Thanks, committed to trunk at r7115. http://gwt-code-reviews.appspot.com/112803 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add deprecation warning to SoycDashboard
Reviewers: Lex,
Description:
Users should no longer use the SoycDashboard directly, because the
compiler option -compileReport will already cause all compile report
files to be written to the extras directory. This patch adds a simple
warning to users of the dashboard.
For people who use -soyc, the particular wording of the warning seems to
imply that they're to use -compileReport if they want the files written
directly. This is a bit misleading, but its only effect will be to move
more people to using -compileReport rather than the deprecated -soyc,
which I think is ok.
Please review this at http://gwt-code-reviews.appspot.com/111803
Affected files:
dev/core/src/com/google/gwt/soyc/SoycDashboard.java
Index: dev/core/src/com/google/gwt/soyc/SoycDashboard.java
===
--- dev/core/src/com/google/gwt/soyc/SoycDashboard.java (revision 7116)
+++ dev/core/src/com/google/gwt/soyc/SoycDashboard.java (working copy)
@@ -65,7 +65,14 @@
}
}
- public static void main(final String[] args) {
+ public static void main(final String[] args) throws InterruptedException
{
+
+System.out.println(WARNING: The direct use of the SoycDashboard is
deprecated and will be removed. +
+ The preferred usage is to invoke the compiler with the
-compileReport option, which +
+ writes the compile report directly to the extra directory.);
+Thread.currentThread();
+Thread.sleep(1000);
+
Settings settings;
try {
settings = Settings.fromArgumentList(args);
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Make compile report handle collapsed permutations (rather than swallowing them)
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? The dashboard now enumerates all permutation descriptions, even when two permutations collapse into one. Thanks, kathrin Please review this at http://gwt-code-reviews.appspot.com/110801 Affected files: dev/core/src/com/google/gwt/core/linker/SoycReportLinker.java dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java dev/core/src/com/google/gwt/soyc/SoycDashboard.java eclipse/samples/Hello/Hello-gwtc.launch -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Updates to SOYC Test to reflect changes in output directory and 1-based counting
Reviewers: Lex,
Description:
Hi Lex,
the previous two patches have implications for SoycTest, which needed to
be updated slightly. This patch does that. Could you review it for me?
Thanks,
kathrin
Please review this at http://gwt-code-reviews.appspot.com/105801
Affected files:
dev/core/test/com/google/gwt/dev/SoycTest.java
Index: dev/core/test/com/google/gwt/dev/SoycTest.java
===
--- dev/core/test/com/google/gwt/dev/SoycTest.java (revision 7021)
+++ dev/core/test/com/google/gwt/dev/SoycTest.java (working copy)
@@ -37,18 +37,19 @@
public void testSoyc() throws UnableToCompleteException, IOException {
options.setSoycEnabled(true);
options.addModuleName(com.google.gwt.sample.hello.Hello);
-options.setWarDir(Utility.makeTemporaryDirectory(null, hellowar));
+
options.setExtraDir(Utility.makeTemporaryDirectory(null, helloextra));
PrintWriterTreeLogger logger = new PrintWriterTreeLogger();
logger.setMaxDetail(TreeLogger.ERROR);
new Compiler(options).run(logger);
// make sure the files have been produced
-assertTrue(new File(options.getWarDir()
+ /hello/compile-report/index.html).exists());
-assertTrue(new File(options.getWarDir()
+ /hello/compile-report/SoycDashboard-0-index.html).exists());
-assertTrue(new File(options.getWarDir()
+ /hello/compile-report/total-0-overallBreakdown.html).exists());
-assertTrue(new File(options.getWarDir()
+ /hello/compile-report/soyc.css).exists());
+System.out.println(extra dir: + options.getExtraDir());
+assertTrue(new File(options.getExtraDir()
+ /hello/soycReport/compile-report/index.html).exists());
+assertTrue(new File(options.getExtraDir()
+ /hello/soycReport/compile-report/SoycDashboard-1-index.html).exists());
+assertTrue(new File(options.getExtraDir()
+ /hello/soycReport/compile-report/total-1-overallBreakdown.html).exists());
+assertTrue(new File(options.getExtraDir()
+ /hello/soycReport/compile-report/soyc.css).exists());
-assertFalse(new File(options.getWarDir()
+ /hello/compile-report/index2.html).exists());
-Util.recursiveDelete(options.getWarDir(), false);
+assertFalse(new File(options.getExtraDir()
+ /hello/soycReport/compile-report/index2.html).exists());
+Util.recursiveDelete(options.getExtraDir(), false);
}
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Yet another SOYC dashboard styling pass
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? It makes a number of changes to the SOYC dashboard, as outlined by John Skidgel in his mock-up. Thanks, kathrin Please review this at http://gwt-code-reviews.appspot.com/91805 Affected files: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java dev/core/src/com/google/gwt/soyc/StaticResources.java dev/core/src/com/google/gwt/soyc/resources/goog.css dev/core/src/com/google/gwt/soyc/resources/images/1bl.gif dev/core/src/com/google/gwt/soyc/resources/images/1br.gif dev/core/src/com/google/gwt/soyc/resources/images/1tl.gif dev/core/src/com/google/gwt/soyc/resources/images/1tr.gif dev/core/src/com/google/gwt/soyc/resources/images/bb.gif dev/core/src/com/google/gwt/soyc/resources/images/blc.gif dev/core/src/com/google/gwt/soyc/resources/images/brc.gif dev/core/src/com/google/gwt/soyc/resources/images/g_gwt.png dev/core/src/com/google/gwt/soyc/resources/images/l.gif dev/core/src/com/google/gwt/soyc/resources/images/r.gif dev/core/src/com/google/gwt/soyc/resources/images/roundedbox_lo.gif dev/core/src/com/google/gwt/soyc/resources/images/roundedbox_lu.gif dev/core/src/com/google/gwt/soyc/resources/images/roundedbox_ro.gif dev/core/src/com/google/gwt/soyc/resources/images/roundedbox_ru.gif dev/core/src/com/google/gwt/soyc/resources/images/tb.gif dev/core/src/com/google/gwt/soyc/resources/images/tlc.gif dev/core/src/com/google/gwt/soyc/resources/images/trc.gif dev/core/src/com/google/gwt/soyc/resources/images/up_arrow.png dev/core/src/com/google/gwt/soyc/resources/inlay.css dev/core/src/com/google/gwt/soyc/resources/soyc.css dev/core/src/com/google/gwt/soyc/resources/soycStyling.css dev/core/test/com/google/gwt/dev/SoycTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: fixes an overly long filename in a compile report
On 2009/10/29 19:52:18, Lex wrote: LTGM. Thanks for taking care of this! http://gwt-code-reviews.appspot.com/90801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Clean up SOYC options
Reviewers: Lex,
Description:
Hi Lex,
could you review this small patch for me? It cleans up the SOYC options
(b/2153332) by:
1) undocumenting -soyc and introducing -compileReport (which is
documented), both of which will set soycEnabled to true.
2) causing -XsoycDetailed to turn on soycEnabled too.
Thanks!
kathrin
Please review this at http://gwt-code-reviews.appspot.com/77826
Affected files:
dev/core/src/com/google/gwt/dev/Precompile.java
dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoyc.java
dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoycDetailed.java
Index: dev/core/src/com/google/gwt/dev/Precompile.java
===
--- dev/core/src/com/google/gwt/dev/Precompile.java (revision 6380)
+++ dev/core/src/com/google/gwt/dev/Precompile.java (working copy)
@@ -43,6 +43,7 @@
import com.google.gwt.dev.util.Memory;
import com.google.gwt.dev.util.PerfLogger;
import com.google.gwt.dev.util.Util;
+import com.google.gwt.dev.util.arg.ArgHandlerCompileReport;
import com.google.gwt.dev.util.arg.ArgHandlerDisableAggressiveOptimization;
import com.google.gwt.dev.util.arg.ArgHandlerDisableCastChecking;
import com.google.gwt.dev.util.arg.ArgHandlerDisableClassMetadata;
@@ -107,6 +108,7 @@
registerHandler(new ArgHandlerMaxPermsPerPrecompile(options));
registerHandler(new ArgHandlerSoyc(options));
registerHandler(new ArgHandlerSoycDetailed(options));
+ registerHandler(new ArgHandlerCompileReport(options));
}
@Override
Index: dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoyc.java
===
--- dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoyc.java
(revision
6380)
+++ dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoyc.java
(working
copy)
@@ -36,6 +36,11 @@
@Override
public String getTag() {
return -soyc;
+ }
+
+ @Override
+ public boolean isUndocumented() {
+return true;
}
@Override
@@ -43,4 +48,5 @@
options.setSoycEnabled(true);
return true;
}
+
}
Index: dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoycDetailed.java
===
--- dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoycDetailed.java
(revision 6380)
+++ dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoycDetailed.java
(working copy)
@@ -15,15 +15,16 @@
*/
package com.google.gwt.dev.util.arg;
+import com.google.gwt.dev.Precompile.PrecompileOptions;
import com.google.gwt.util.tools.ArgHandlerFlag;
/**
* An ArgHandler that enables detailed Story Of Your Compile data
collection.
*/
public class ArgHandlerSoycDetailed extends ArgHandlerFlag {
- private final OptionSoycDetailed options;
+ private final PrecompileOptions options;
- public ArgHandlerSoycDetailed(OptionSoycDetailed options) {
+ public ArgHandlerSoycDetailed(PrecompileOptions options) {
this.options = options;
}
@@ -45,6 +46,7 @@
@Override
public boolean setFlag() {
options.setSoycExtra(true);
+options.setSoycEnabled(true);
return true;
}
}
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] test case for -soyc and SOYC dashboard
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? It implements a simple test case that invokes the compiler with the -soyc flag, produces the dashboard files, and then checks whether the files exist. Getting this to work also required some build file magic (thanks to jlabanca). Thanks! Please review this at http://gwt-code-reviews.appspot.com/77815 Affected files: dev/core/build.xml dev/core/test/com/google/gwt/dev/CompilerTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] right-click for tree items
Reviewers: jlabanca,
Description:
Hi John,
could you review this patch for me? It's very small.
The problem we were seeing was that for tree items, right- and
middle-clicks were handled and the associated event were fired. This
patch checks that only left-clicks fire such events.
Thanks!
kathrin
Please review this at http://gwt-code-reviews.appspot.com/78813
Affected files:
user/src/com/google/gwt/user/client/ui/Tree.java
Index: user/src/com/google/gwt/user/client/ui/Tree.java
===
--- user/src/com/google/gwt/user/client/ui/Tree.java(revision 6346)
+++ user/src/com/google/gwt/user/client/ui/Tree.java(working copy)
@@ -541,7 +541,9 @@
// Currently, the way we're using image bundles causes extraneous
events
// to be sunk on individual items' open/close images. This leads
to an
// extra event reaching the Tree, which we will ignore here.
-if (DOM.eventGetCurrentTarget(event) == getElement()) {
+// Also, ignore middle and right clicks here.
+if ((DOM.eventGetCurrentTarget(event) == getElement())
+ (event.getButton() == Event.BUTTON_LEFT)) {
elementClicked(DOM.eventGetTarget(event));
}
break;
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: changes to hyperlink and Showcase sample
Thanks to both of you! Committed to branch at r6278.
http://gwt-code-reviews.appspot.com/74801/diff/1002/1006
File
samples/showcase/src/com/google/gwt/sample/showcase/server/CrawlServlet.java
(right):
http://gwt-code-reviews.appspot.com/74801/diff/1002/1006#newcode37
Line 37: * Servlet that makes this application crawlable.
On 2009/09/30 20:48:34, zundel wrote:
On 2009/09/30 20:38:21, kathrin wrote:
On 2009/09/30 14:09:43, zundel wrote:
I don't see anything in here that looks showcase specific. Is
there a plan
to
bundle this up as a library?
Good question. The plan was to ship a modified app that serves as a
template
for other applications, and the CrawlServlet would be part of this
sample app.
How do you think we could bundle this up as a library?
I dunno, I was just thinking we might include this class in GWT proper
and
bundle it up along with the gwt-servlet distro. Then, could anyone
just throw
it in their web.xml?
This sounds like a good idea. I'll leave this as a to-do for now until
I figure out where it should best go, etc.
http://gwt-code-reviews.appspot.com/74801/diff/1015/31#newcode45
Line 45: if (i == -1) {
On 2009/09/30 21:01:41, Dan Rice wrote:
comment
Done.
http://gwt-code-reviews.appspot.com/74801/diff/1015/31#newcode46
Line 46: i = queryStringSb.indexOf(?_escaped_fragment_=);
On 2009/09/30 21:01:41, Dan Rice wrote:
comment
Done.
http://gwt-code-reviews.appspot.com/74801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] changes to hyperlink and Showcase sample
Reviewers: Dan Rice, Description: Hi Dan, could you review this patch for me? It implements changes to the Showcase sample and to hyperlink. 1) Showcase: Showcase needs to have indexable hyperlinks (see below) and needs changes to web.xml as well as an added CrawlServlet that invokes the headless browser when needed. 2) Hyperlink: I've added an additional hyperlink class IndexableHyperlink. This addition should not break anybody who currently uses hyperlinks. Thanks, kathrin Please review this at http://gwt-code-reviews.appspot.com/74801 Affected files: eclipse/README.txt eclipse/samples/Showcase/.classpath samples/showcase/src/com/google/gwt/sample/showcase/client/Showcase.java samples/showcase/src/com/google/gwt/sample/showcase/server/CrawlServlet.java samples/showcase/war/Showcase.html samples/showcase/war/WEB-INF/web.xml servlet/build.xml user/src/com/google/gwt/user/client/History.java user/src/com/google/gwt/user/client/impl/HistoryImpl.java user/src/com/google/gwt/user/client/ui/IndexableHyperlink.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] overhaul of styling in SOYC dashboard
Reviewers: Lex, Description: Hi Lex, could you review this patch for me? This patch overhauls styling in the SOYC dashboard. It aims at following styling guidelines by Google, including the color scheme, which is modeled after trends.google.com. It does this by getting rid of most of the styling we have had (including the rounded corner images). It also overhauls MakeTopLevelHtmlForPerm, greatly simplifying it. Please note that this patch will need to be merged with your recent merge which will likely go into trunk first. Please review this at http://gwt-code-reviews.appspot.com/74802 Affected files: dev/common.ant.xml dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java tools/soyc-vis/build.xml tools/soyc-vis/classLevel.css tools/soyc-vis/common.css tools/soyc-vis/images/1bl.gif tools/soyc-vis/images/1br.gif tools/soyc-vis/images/1tl.gif tools/soyc-vis/images/1tr.gif tools/soyc-vis/images/bb.gif tools/soyc-vis/images/blc.gif tools/soyc-vis/images/brc.gif tools/soyc-vis/images/l.gif tools/soyc-vis/images/r.gif tools/soyc-vis/images/roundedbox_lo.gif tools/soyc-vis/images/roundedbox_lu.gif tools/soyc-vis/images/roundedbox_ro.gif tools/soyc-vis/images/roundedbox_ru.gif tools/soyc-vis/images/tb.gif tools/soyc-vis/images/tlc.gif tools/soyc-vis/images/trc.gif tools/soyc-vis/roundedCorners.css tools/soyc-vis/soycStyling.css --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Simple crawler
Thanks, committed to crawlability branch at r6204. http://gwt-code-reviews.appspot.com/70802/diff/24/28 File tools/simple-crawler/src/com/google/gwt/simplecrawler/Settings.java (right): http://gwt-code-reviews.appspot.com/70802/diff/24/28#newcode102 Line 102: * Processes the arguments from the command line On 2009/09/24 14:57:54, Dan Rice wrote: period Done. http://gwt-code-reviews.appspot.com/70802/diff/24/28#newcode140 Line 140: * Displays usage information On 2009/09/24 14:57:54, Dan Rice wrote: period Done. http://gwt-code-reviews.appspot.com/70802/diff/24/29 File tools/simple-crawler/src/com/google/gwt/simplecrawler/SimpleCrawler.java (right): http://gwt-code-reviews.appspot.com/70802/diff/24/29#newcode136 Line 136: * _escaped_fragment_=. E.g., www.example.com#!mystate is mapped to On 2009/09/24 14:57:54, Dan Rice wrote: For example, Done. http://gwt-code-reviews.appspot.com/70802/diff/24/29#newcode155 Line 155: * Maps back to the original URL, which can contain #!. E.g., On 2009/09/24 14:57:54, Dan Rice wrote: For example, Done. http://gwt-code-reviews.appspot.com/70802/diff/24/29#newcode177 Line 177: // hrefs On 2009/09/24 14:57:54, Dan Rice wrote: Use multi-line comment style: /* * Blah * Blah Blah */ Done. http://gwt-code-reviews.appspot.com/70802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Simple crawler
Reviewers: Dan Rice, Description: Could you review this patch for me? This simple crawler is intended to be used by users who crawl-enable their apps. It gives them an idea of what the crawler will see, without making any guarantees about emulating the real google crawler. It takes in either a sitemap file (get one from http://j15r.com:8800/Showcase/Sitemap.xml) or a URL (e.g., http://j15r.com:8800/Showcase). Optionally, it also takes an output file (by default, it prints to stdout). Please review this at http://gwt-code-reviews.appspot.com/70802 Affected files: eclipse/tools/simple-crawler/.checkstyle eclipse/tools/simple-crawler/.classpath eclipse/tools/simple-crawler/.project tools/simple-crawler/build.xml tools/simple-crawler/src/com/google/gwt/crawler/Settings.java tools/simple-crawler/src/com/google/gwt/crawler/SimpleCrawler.java tools/simple-crawler/src/com/google/gwt/crawler/SimpleSitemapParser.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add permutation info to index page for each permutation
On 2009/09/10 15:42:26, Lex wrote: LGTM Thanks, committed at r6138. http://gwt-code-reviews.appspot.com/64808 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Add permutation info to index page for each permutation
Reviewers: ,
Description:
Hi Lex,
could you review this patch for me? It follows up on a (very useful!)
feature request by Adam to add permutation information to the index page
for each individual permutation.
Thanks,
kathrin
Please review this at http://gwt-code-reviews.appspot.com/64808
Affected files:
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
Index: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
===
--- dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(revision
6077)
+++ dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(working
copy)
@@ -856,7 +856,12 @@
outFile.println(body);
outFile.println(div class='abs mainHeader');
outFile.println(h2Story of Your Compile Dashboard/h2);
-
+String permutationInfo = settings.allPermsInfo.get(permutationId);
+outFile.print(h3Permutation + permutationId);
+if (permutationInfo.length() 0) {
+ outFile.println( ( + permutationInfo + ));
+}
+outFile.println(/h3);
outFile.println(hr);
outFile.println(center);
if (globalInformation.getSplitPointToLocation().size() 1) {
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Eliminate dead links in SOYC dashboard when displayDependencies is off
On 2009/09/08 15:05:18, Lex wrote: LGTM. Good catch! Thanks, committed at r6096. http://gwt-code-reviews.appspot.com/65803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Change SOYC dashboard to accept symbol maps created by different linker
On 2009/09/03 16:14:54, kathrin wrote: Thanks, Lex, for a desk review. Committed at r6098. http://gwt-code-reviews.appspot.com/65802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Change SOYC dashboard to accept symbol maps created by different linker
Reviewers: Lex, schuck_google.com,
Description:
This change to the dashboard updates the settings parser to read symbol
maps files created with the CompactSymbolLinker.
Please review this at http://gwt-code-reviews.appspot.com/65802
Affected files:
dev/core/src/com/google/gwt/soyc/Settings.java
Index: dev/core/src/com/google/gwt/soyc/Settings.java
===
--- dev/core/src/com/google/gwt/soyc/Settings.java (revision 6077)
+++ dev/core/src/com/google/gwt/soyc/Settings.java (working copy)
@@ -230,16 +230,19 @@
while ((sc.hasNextLine()) (lineCount 2)) {
String curLine = sc.nextLine();
- curLine = curLine.replace(# {, );
- curLine = curLine.replace(}, );
curLine = curLine.trim();
- if (lineCount == 0) {
-permutationId = curLine;
- } else {
-permutationInfo = curLine;
+ if (curLine.startsWith(# {)) {
+curLine = curLine.replace(# {, );
+curLine = curLine.replace(}, );
+curLine = curLine.trim();
+if (lineCount == 0) {
+ permutationId = curLine;
+} else {
+ permutationInfo = curLine;
+}
+lineCount++;
}
- lineCount++;
}
allPermsInfo.put(permutationId, permutationInfo);
}
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Eliminate dead links in SOYC dashboard when displayDependencies is off
Reviewers: Lex,
Description:
Hi Lex,
could you review this patch for me? It eliminates some dead links in
the dashboard when displayDependencies is turned off.
Thanks!
kathrin
Please review this at http://gwt-code-reviews.appspot.com/65803
Affected files:
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
Index: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
===
--- dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(revision
6077)
+++ dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(working
copy)
@@ -729,7 +729,7 @@
outFile.println(tr);
outFile.println(td class=\barlabel\ + size + /td);
outFile.println(td class=\barlabel\ + perc + %/td);
-if (dependencyLink != null) {
+if ((settings.displayDependencies) (dependencyLink != null)) {
outFile.println(td class=\barlabel\a href=\ +
dependencyLink
+ \ target=\_top\ + className + /a/td);
} else {
@@ -1007,15 +1007,17 @@
private void addLefttoversStatus(String className, String packageName,
PrintWriter out, String permutationId) {
-out.println(trtdnbsp;nbsp;nbsp;nbsp;a href=\
-+ dependenciesFileName(total, packageName, permutationId) + #
-+ className + \See why it's live/a/td/tr);
-for (int sp = 1; sp = globalInformation.getNumSplitPoints(); sp++) {
+if (settings.displayDependencies) {
out.println(trtdnbsp;nbsp;nbsp;nbsp;a href=\
- + dependenciesFileName(sp + sp, packageName, permutationId)
+ #
- + className + \See why it's not exclusive to s.p. # + sp +
(
- + globalInformation.getSplitPointToLocation().get(sp)
- + )/a/td/tr);
+ + dependenciesFileName(total, packageName, permutationId) + #
+ + className + \See why it's live/a/td/tr);
+ for (int sp = 1; sp = globalInformation.getNumSplitPoints(); sp++) {
+out.println(trtdnbsp;nbsp;nbsp;nbsp;a href=\
++ dependenciesFileName(sp + sp, packageName, permutationId)
+ #
++ className + \See why it's not exclusive to s.p. # + sp
+ (
++ globalInformation.getSplitPointToLocation().get(sp)
++ )/a/td/tr);
+ }
}
}
@@ -1386,9 +1388,14 @@
out.println(table border=\1\ width=\80%\ style=\font-size:
11pt;\ bgcolor=\white\);
if
(globalInformation.getInitialCodeBreakdown().classToSize.containsKey(className))
{
- out.println(trtdSome code is initial (a href=\
- + dependenciesFileName(initial, packageName, permutationId)
+ #
- + className + \see why/a)/td/tr);
+ if (settings.displayDependencies) {
+out.println(trtdSome code is initial (a href=\
++ dependenciesFileName(initial, packageName, permutationId)
+ #
++ className + \see why/a)/td/tr);
+ }
+ else {
+out.println(trtdSome code is initial/td/tr);
+ }
}
for (int sp : splitPointsWithClass(className)) {
out.println(trtdSome code downloads with s.p. # + sp + (
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Fix path issue in SoycDashboard after moving soyc resources to a sub-directory
Reviewers: Lex,
Description:
Hi Lex,
could you review this tiny patch for me? It fixes a problem we had
introduced by moving all SOYC resources to a subdirectory: when people
were using the -resources flag to actually point at the directory that
contains the css and image files, the dashboard would fail. With this
patch, we would allow both possible paths.
Thanks,
kathrin
Please review this at http://gwt-code-reviews.appspot.com/61815
Affected files:
dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
Index: dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
===
--- dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(revision
6018)
+++ dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(working
copy)
@@ -281,24 +281,36 @@
}
String inputFileName = roundedCorners.css;
File inputFile = new File(classPath + RESOURCES_PATH + inputFileName);
+if (!inputFile.exists()) {
+ inputFile = new File(classPath + inputFileName);
+}
File outputFile = getOutFile(roundedCorners.css);
copyFileOrDirectory(inputFile, outputFile, classPath, RESOURCES_PATH
+ inputFileName, false);
inputFileName = classLevel.css;
File inputFile2 = new File(classPath + RESOURCES_PATH + inputFileName);
+if (!inputFile2.exists()) {
+ inputFile2 = new File(classPath + inputFileName);
+}
File outputFile2 = getOutFile(classLevel.css);
copyFileOrDirectory(inputFile2, outputFile2, classPath, RESOURCES_PATH
+ inputFileName, false);
inputFileName = common.css;
File inputFile3 = new File(classPath + RESOURCES_PATH + inputFileName);
+if (!inputFile3.exists()) {
+ inputFile3 = new File(classPath + inputFileName);
+}
File outputFile3 = getOutFile(common.css);
copyFileOrDirectory(inputFile3, outputFile3, classPath, RESOURCES_PATH
+ inputFileName, false);
inputFileName = images;
File inputDir = new File(classPath + RESOURCES_PATH + images);
+if (!inputDir.exists()) {
+ inputDir = new File(classPath + images);
+}
File outputDir = getOutFile(images);
copyFileOrDirectory(inputDir, outputDir, classPath, inputFileName,
true);
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Call webClient.closeAllWindows() in htmlunit code
Hi Amit, I think this is the right direction! But I do think there's still a problem here, if I read the HtmlUnit documentation correctly. It says: - waitForBackgroundJavaScriptStartingBefore This method blocks until all background JavaScript tasks scheduled to start executing before (now + delayMillis) have finished executing. Background JavaScript tasks are JavaScript tasks scheduled for execution via window.setTimeout, window.setInterval or asynchronous XMLHttpRequest. ... Returns: the number of background JavaScript jobs still executing or waiting to be executed when this method returns; will be 0 if there are no jobs left to execute - Now what's happening here is that you wait x ms for the jobs to finish, but then there will at least be 1 still running (because GWT apps have a setTimeout(200?) that keeps setting a setTimeout(200) to check whether the history has changed. So unless I'm missing something, every time this method returns, it'll return a value 0, which means you'll just loop around until the test times out. Am I missing something? http://gwt-code-reviews.appspot.com/62802/diff/1/2 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/62802/diff/1/2#newcode101 Line 101: + JAVASCRIPT_WAIT_TIME + ms); Rephrase a bit? Maybe say Waiting for (++count) javascript jobs for xxx ms http://gwt-code-reviews.appspot.com/62802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Call webClient.closeAllWindows() in htmlunit code
In a discussion with Amit and Joel, we've determined that the tests that this will apply to do not use History tokens, so the repeated timeout won't apply. We couldn't think of any other cases where there would be lingering JavaScript jobs. TODO - we should revisit this issue at some point and come up with something more stable. On 2009/08/20 20:27:25, kathrin wrote: Hi Amit, I think this is the right direction! But I do think there's still a problem here, if I read the HtmlUnit documentation correctly. It says: - waitForBackgroundJavaScriptStartingBefore This method blocks until all background JavaScript tasks scheduled to start executing before (now + delayMillis) have finished executing. Background JavaScript tasks are JavaScript tasks scheduled for execution via window.setTimeout, window.setInterval or asynchronous XMLHttpRequest. ... Returns: the number of background JavaScript jobs still executing or waiting to be executed when this method returns; will be 0 if there are no jobs left to execute - Now what's happening here is that you wait x ms for the jobs to finish, but then there will at least be 1 still running (because GWT apps have a setTimeout(200?) that keeps setting a setTimeout(200) to check whether the history has changed. So unless I'm missing something, every time this method returns, it'll return a value 0, which means you'll just loop around until the test times out. Am I missing something? http://gwt-code-reviews.appspot.com/62802/diff/1/2 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/62802/diff/1/2#newcode101 Line 101: + JAVASCRIPT_WAIT_TIME + ms); Rephrase a bit? Maybe say Waiting for (++count) javascript jobs for xxx ms http://gwt-code-reviews.appspot.com/62802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Illegal XML characters in SOYC XML files
Thanks, Lex. I didn't try the #x; idea (see Ian's comment), but I also added the other illegal characters. I'll leave the recoverability (in the dashboard) for another day: (x00) and (u) seem good to me for human consumption, and the surrogate blocks characters shouldn't really ever in an application. On 2009/08/19 13:28:10, Lex wrote: I like the #x; idea. There is just one potential problem: will XML readers support it? The linked XML spec has the same restrictions on encoded character entities as on raw characters appearing in the file. Does anyone know if that restriction is honored in practice? Anyone want to test on Xerces? http://gwt-code-reviews.appspot.com/61801 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Illegal XML characters in SOYC XML files
Reviewers: Lex,
Description:
Hi Lex,
would you review this simple patch for me? It catches the illegal XML
characters that were throwing off the SAX parser.
TODO: there are still some character that could theoretically sneak into
our xml files, see http://www.w3.org/TR/REC-xml/#charsets. This should
be fixed.
Thanks,
kathrin
Please review this at http://gwt-code-reviews.appspot.com/61801
Affected files:
dev/core/src/com/google/gwt/dev/util/Util.java
Index: dev/core/src/com/google/gwt/dev/util/Util.java
===
--- dev/core/src/com/google/gwt/dev/util/Util.java (revision 5969)
+++ dev/core/src/com/google/gwt/dev/util/Util.java (working copy)
@@ -317,7 +317,7 @@
* equivalents. The portion of the input string between start
(inclusive) and
* end (exclusive) is scanned. The output is appended to the given
* StringBuilder.
- *
+ *
* @param code the input String
* @param start the first character position to scan.
* @param end the character position following the last character to
scan.
@@ -330,7 +330,7 @@
int lastIndex = 0;
int len = end - start;
char[] c = new char[len];
-
+
code.getChars(start, end, c, 0);
for (int i = 0; i len; i++) {
switch (c[i]) {
@@ -361,6 +361,16 @@
lastIndex = i + 1;
}
break;
+case '\0':
+ builder.append(c, lastIndex, i - lastIndex);
+ builder.append((null));
+ lastIndex = i + 1;
+ break;
+case '\u':
+ builder.append(c, lastIndex, i - lastIndex);
+ builder.append((\u));
+ lastIndex = i + 1;
+ break;
default:
break;
}
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] clean up compiler logging output
Reviewers: Lex, bruce, Description: Hi Lex Bruce, could you review this patch for me? It cleans up the compiler logging output. In particular, it eliminates showing SOYC logging by default (even when there is no -soyc). It also shows the absolute path of the war directory where compiler output is stored. By default, compiling Hello now shows, which I believe strikes a nice balance: Compiling module com.google.gwt.sample.hello.Hello Compiling 6 permutations Compiling permutation 1... Compiling permutation 2... Compiling permutation 3... Compiling permutation 4... Compiling permutation 5... Compiling permutation 6... Compile of permutations succeeded Linking into /Users/kprobst/work/gwt-trunk-soyc/eclipse/samples/Hello/war/hello Link succeeded Compilation succeeded -- 17.314s Please review this at http://gwt-code-reviews.appspot.com/60802 Affected files: dev/core/src/com/google/gwt/core/ext/soyc/impl/DependencyRecorder.java dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java dev/core/src/com/google/gwt/dev/CompilePerms.java dev/core/src/com/google/gwt/dev/Compiler.java dev/core/src/com/google/gwt/dev/GWTCompiler.java dev/core/src/com/google/gwt/dev/PermutationWorkerFactory.java dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: move SOYC resources out of the top-level jar directory
LGTM http://gwt-code-reviews.appspot.com/60803/diff/1/2 File tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java (right): http://gwt-code-reviews.appspot.com/60803/diff/1/2#newcode126 Line 126: + /resources/; Just wondering why you infer the path here (you hard-code it in the build file). It's fine, but I suppose you could just hard code it here too. http://gwt-code-reviews.appspot.com/60803 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: SOYC for multiple permutations, plus updated layout
Thanks for the feedback! I've fixed the following in the updated patch:
1) backwards compatibility
2) make GlobalInformation contain no static fields. GlobalInformation
is now re-created for each iteration. Truly global information (e.g.,
permutation information strings) are now moved to Settings.
And I sneaked in a couple of other fixes:
1) Handle both .xml.gz and .xml, as in the previous version.
2) Fix a small problem where the dashboard would crash if the specified
-out directory did not yet exist.
3) Some checkstyle fixes, e.g., adding some javadoc comments.
On 2009/08/12 15:18:37, Lex wrote:
These are both great changes!
I believe we should figure out the upgrade path, though, before
committing the
part about multi-permutation support. There are a couple of
strategies
described below, or maybe you can think of another one. One way or
another, the
goal is to find a way to support the new behavior without breaking
existing
build scripts.
http://gwt-code-reviews.appspot.com/57813/diff/1/4
File tools/soyc-vis/src/com/google/gwt/soyc/GlobalInformation.java
(right):
http://gwt-code-reviews.appspot.com/57813/diff/1/4#newcode142
Line 142: public static void reset() {
This method is necessary for now, but it really makes me want to move
to
non-static variables. We will then be able to simply discard the
global
information about the last permutation and create a new one for the
next
permutation.
http://gwt-code-reviews.appspot.com/57813/diff/1/4#newcode149
Line 149: initialCodeBreakdown.classToSize.clear();
Speaking of which, it would be cleaner here to reassign
initialCodeBreakdown,
etc., instead of clearing all their fields.
http://gwt-code-reviews.appspot.com/57813/diff/1/2
File tools/soyc-vis/src/com/google/gwt/soyc/Settings.java (right):
http://gwt-code-reviews.appspot.com/57813/diff/1/2#newcode125
Line 125: throw new ArgumentListException(Must specify the soyc
directory);
There is a problem here. The new argument handling will break
existing scripts
which specify one to three xml.gz files.
Upgrading is much easier if we modify the argument handling to
tolerate what
existing scripts throw at this class. In general that might be a
problem, but
in this case I see two ways that aren't too bad. Maybe there's an
even better
way you can think of.
One way I see would be to do an isDirectory test on the remaining
arguments. If
they are directories, use the new stuff; if they are files, then use
the old
stuff, assuming just one permutation. Another way would be to add
options
-soycDir and -symbolMapsDir; the xml.gz files are then required if
these
arguments are not present.
http://gwt-code-reviews.appspot.com/57813/diff/1/5
File tools/soyc-vis/src/com/google/gwt/soyc/SizeBreakdown.java
(right):
http://gwt-code-reviews.appspot.com/57813/diff/1/5#newcode26
Line 26: public void initializeLiteralsCollection(
I believe this file will no longer need to be changed, if
GlobalInformation.reset() discards the old objects and creates new
ones.
http://gwt-code-reviews.appspot.com/57813
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] SOYC for multiple permutations, plus updated layout
Reviewers: Lex, Description: Hi Lex, would you review this patch to the SOYC dashboard for me? It has two main parts: 1) SOYC for multiple permutations. The dashboard will now read all files in the symbolMaps directory and from it derive all the permutations and their attributes. The dashboard will show a front page that lists (and links to) all permutations. 2) This patch updates the dashboard according to Joel's new layout mockup. The next steps are summarized in follow-up issues 2048346 (more info on front page) and 2048348 (remaining layout issues). Time permitting, we can also avoid some code reduplication in MakeTopLevelHtmlForPerm.java. Please review this at http://gwt-code-reviews.appspot.com/57813 Affected files: tools/soyc-vis/src/com/google/gwt/soyc/GlobalInformation.java tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java tools/soyc-vis/src/com/google/gwt/soyc/Settings.java tools/soyc-vis/src/com/google/gwt/soyc/SizeBreakdown.java tools/soyc-vis/src/com/google/gwt/soyc/SoycDashboard.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: faster size breakdown with non-fractional billing
Here are the inline code comments.
http://gwt-code-reviews.appspot.com/51804/diff/1/8
File
dev/core/src/com/google/gwt/core/ext/linker/impl/StandardCompilationAnalysis.java
(right):
http://gwt-code-reviews.appspot.com/51804/diff/1/8#newcode57
Line 57: */
Would it make sense to refer to these as size maps throughout?
http://gwt-code-reviews.appspot.com/51804/diff/1/21
File dev/core/src/com/google/gwt/dev/Precompile.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/21#newcode57
Line 57: import com.google.gwt.dev.util.arg.ArgHandlerSoycExtra;
same question as in JJSOptions.
http://gwt-code-reviews.appspot.com/51804/diff/1/15
File dev/core/src/com/google/gwt/dev/jjs/JJSOptions.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/15#newcode36
Line 36: OptionSoycEnabled, OptionSoycExtra,
OptionCompilationStateRetained, OptionOptimizePrecompile {
could this be combined with OptionSoycEnabled (and reasonable defaults)?
http://gwt-code-reviews.appspot.com/51804/diff/1/16
File dev/core/src/com/google/gwt/dev/jjs/JJSOptionsImpl.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/16#newcode35
Line 35: private boolean soycExtra = false;
Same question as in JJSOptions.java - could we just have one soyc
option? It might seem a bit confusing to the user to have to choose
settings for two options. Maybe -soyc could default to this new output
(with size maps), and -soyc detailed could give the detailed
information?
http://gwt-code-reviews.appspot.com/51804/diff/1/18
File dev/core/src/com/google/gwt/dev/js/JsReportGenerationVisitor.java
(right):
http://gwt-code-reviews.appspot.com/51804/diff/1/18#newcode39
Line 39: super(out, map);
(note kprobst) - do we now always need to pass in the map? does this
make sense without soyc?
http://gwt-code-reviews.appspot.com/51804/diff/1/22
File dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerSoycExtra.java
(right):
http://gwt-code-reviews.appspot.com/51804/diff/1/22#newcode37
Line 37: return -XsoycExtra;
maybe -XsoycDetailed ?
http://gwt-code-reviews.appspot.com/51804/diff/1/3
File tools/soyc-vis/src/com/google/gwt/soyc/CodeCollection.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/3#newcode34
Line 34: codeType = type;
Similar to LiteralsCollection, do we still need the type in this class?
http://gwt-code-reviews.appspot.com/51804/diff/1/4
File tools/soyc-vis/src/com/google/gwt/soyc/GlobalInformation.java
(right):
http://gwt-code-reviews.appspot.com/51804/diff/1/4#newcode87
Line 87: private static void computePartialPackageSizes(
Why not just call this computePackageSizes?
http://gwt-code-reviews.appspot.com/51804/diff/1/4#newcode89
Line 89: float cumPartialSizeFromPackages = 0f;
This doesn't seem to be used.
http://gwt-code-reviews.appspot.com/51804/diff/1/5
File tools/soyc-vis/src/com/google/gwt/soyc/SizeBreakdown.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/5#newcode55
Line 55: public MapString, Float packageToSize = new HashMapString,
Float();
I'm not sure why we need Floats for sizes now that we don't do partial
billing any more.
http://gwt-code-reviews.appspot.com/51804/diff/1/2
File tools/soyc-vis/src/com/google/gwt/soyc/SoycDashboard.java (right):
http://gwt-code-reviews.appspot.com/51804/diff/1/2#newcode383
Line 383: // variables
We might not want to use random variables here - overloaded
terminology.
http://gwt-code-reviews.appspot.com/51804
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: SOYC filenames all end with .html
LGTM http://gwt-code-reviews.appspot.com/54813 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: SOYC generates more dependency trees
On 2009/07/01 17:56:26, Lex wrote: Generally, LGTM. I had a few comments below. Going forward, I notice that SOYC + the dashboard are growing in complexity and functionality - there is nothing wrong with that, and it's definitely a good step forward. However, would it make sense to write up a number of questions that the user may ask, and then design the UI that way? For instance, the new patch includes answers to questions ranging from what is downloaded in a typical initial load sequence? to what split points include code belonging to a specific class? Both of these are very good questions, but they are very different ways of looking at SOYC output. I see two options: a) a full-blown query language (far off, but would be very cool), or b) a UI driven by use cases and/or specific questions, which we spell out explicitly for the user, either in the UI itself or else in an accompaying doc. What do you think? http://gwt-code-reviews.appspot.com/50806 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Manifest for SOYC permutations
Reviewers: Lex, Description: This patch creates a manifest file for each compilation permutation if the compilation is SOYC enabled. The manifest files are stored alongside other SOYC manifests (in the soycReport directory). Each manifest file contains all non-gwt properties for the permutation, i.e., user agent, locale, etc. Please review this at http://gwt-code-reviews.appspot.com/50801 Affected files: dev/core/src/com/google/gwt/core/ext/linker/CompilationAnalysis.java dev/core/src/com/google/gwt/core/ext/linker/impl/StandardCompilationAnalysis.java dev/core/src/com/google/gwt/core/ext/soyc/impl/ManifestRecorder.java dev/core/src/com/google/gwt/core/linker/SoycReportLinker.java dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: SOYC gives drill-down on size for leftover and exclusive fragments
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
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Speedups for -soyc compilation
LGTM I do want to re-iterate Lex's point that we need to make sure that the dashboard does not produce different results with this. This is especially true because we know that the output of the SOYC files has changed with this patch. Having said that, this should really speed up compilation, and as a nice side effect, the resulting files should be quite a bit smaller, which is a VERY good thing. http://gwt-code-reviews.appspot.com/34818/diff/1002/1003 File dev/core/src/com/google/gwt/core/ext/soyc/Story.java (right): http://gwt-code-reviews.appspot.com/34818/diff/1002/1003#newcode35 Line 35: import java.util.SortedSet; do you still need this import? http://gwt-code-reviews.appspot.com/34818/diff/1002/1004 File dev/core/src/com/google/gwt/core/ext/soyc/impl/StoryImpl.java (right): http://gwt-code-reviews.appspot.com/34818/diff/1002/1004#newcode24 Line 24: import java.util.SortedSet; again, delete this import. http://gwt-code-reviews.appspot.com/34818/diff/1002/1008 File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/34818/diff/1002/1008#newcode368 Line 368: + (System.currentTimeMillis() - permStart) + ms); I'm not sure we want to have this in the general compile. If you want it in there, add it to a logger instead of printing to stdout? http://gwt-code-reviews.appspot.com/34818/diff/1002/1010 File dev/core/src/com/google/gwt/dev/jjs/SourceInfo.java (right): http://gwt-code-reviews.appspot.com/34818/diff/1002/1010#newcode66 Line 66: SetCorrelation getPrimaryCorrelations(); can we get rid of this (now that we have Correlation[] get PrimaryCorrelationsArray())? I know this gets called from other places as well, but it would be good to simplify. If it you get rid of it, can you do the same for Correlation get PrimaryCorrelation(Axis axis)? http://gwt-code-reviews.appspot.com/34818/diff/1002/1011 File dev/core/src/com/google/gwt/dev/jjs/SourceInfoCorrelation.java (right): http://gwt-code-reviews.appspot.com/34818/diff/1002/1011#newcode40 Line 40: private static final int numCorrelationAxes = Axis.values().length; I'd be a bit careful with this. It's a nice simplification, but SOYC compiles tend to suffer from bad memory blow-ups. This will add a field to every single SourceOrigin. http://gwt-code-reviews.appspot.com/34818 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Speed up escapeXml methods
LGTM Like you mentioned, with a more intrusive change we might (in the future) be able to get away without ever converting to/from String. http://gwt-code-reviews.appspot.com/34814 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: add -out to SoycDashboard
On 2009/05/18 21:56:02, spoon wrote: LGTM http://gwt-code-reviews.appspot.com/33825 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Addresses issues 1820736 and 1843669 (Soyc dashboard improvements)
Reviewers: spoon, Please review this at http://gwt-code-reviews.appspot.com/33822 Affected files: tools/soyc-vis/src/com/google/gwt/soyc/GlobalInformation.java tools/soyc-vis/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java tools/soyc-vis/src/com/google/gwt/soyc/Settings.java tools/soyc-vis/src/com/google/gwt/soyc/SoycDashboard.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
