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
