[+cc google-web-toolkit-contributors]
I don't know why I've been added as a reviewer here, but here are a few
comments. (beware, I'm not in the GWT Team)
http://gwt-code-reviews.appspot.com/1454804/diff/7002/user/src/com/google/gwt/uibinder/elementparsers/AbsolutePanelParser.java
File
Patchset #2:
Extracts isValidUriCharset from isValidUri, with a specific test.
Adds test cases to test suites.
Adds a few tests and fixes another few (including a risk of false
positives in SafeHtmlHostedModeUtilsTest).
Fixed copyright dates on a the new files.
Note that I didn't
committed as r10264
http://gwt-code-reviews.appspot.com/1452803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1453805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10305
Author: jasonh...@google.com
Date: Thu Jun 9 04:36:38 2011
Log: HTML5 Geolocation support in GWT
Review at http://gwt-code-reviews.appspot.com/1451811
Review by: jlaba...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10305
Added:
http://gwt-code-reviews.appspot.com/1449814/diff/5/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
File user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
(right):
Justin, I really appreciate the contribution, and this is a nice
mechanism. But the reason we haven't done something like this already is
that we don't want to lock down the api to the parsers.
In fact at the moment it's in a lot of flux as we add features to
support Cell authors, and as we try
LGTM.
Also, jlabanca kindly offered to submit this patch for you.
Thanks!
--xtof
http://gwt-code-reviews.appspot.com/1449814/diff/5/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
File user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/1453805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Ready for another look.
http://gwt-code-reviews.appspot.com/1447815/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Looking.
Dan, can you review this too?
http://gwt-code-reviews.appspot.com/1447815/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10306
Author: p...@google.com
Date: Thu Jun 9 05:39:07 2011
Log: Rollback 100 column format changes (issue 1450808 and issue
1447822)
The new format changes don't work with our current header checkstyle checks
and the fix isn't trivial. Until I have that ready, I'm
Revision: 10307
Author: zun...@google.com
Date: Thu Jun 9 05:46:38 2011
Log: Exercise RunAsyncCode class for prefetching fragments.
Recent changes to code splitting fixed a bug in RunAsyncCode.isLoaded().
I wanted to write a test to see that things were really working.
Review at
I'll take a close look next week and get this submitted.
http://gwt-code-reviews.appspot.com/1449814/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1447815/diff/65/user/src/com/google/web/bindery/requestfactory/apt/RfApt.java
File user/src/com/google/web/bindery/requestfactory/apt/RfApt.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/1447815/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: tobyr, rdayal, Jason Parekh,
Description:
Add JSO inspection support.
IDEs can use this class to implement JSO inspection capabilities in
their debug view.
Please review this at http://gwt-code-reviews.appspot.com/1454807/
Affected files:
A
http://gwt-code-reviews.appspot.com/1454801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
(right):
Reviewers: rchandia,
Description:
Adding Column#set/getCellStyleNames() to specify style names to apply to
individual cells of a CellTable/DataGrid. Users can set a general style
name to apply to the column, or override getCellStyleNames() to provide
style names based on the row/cell value.
I
LGTM
http://gwt-code-reviews.appspot.com/1454807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1446817/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1446817/diff/1/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
File user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/1446817/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: rchandia,
Description:
Better error message for ui:attribute
Please review this at http://gwt-code-reviews.appspot.com/1454809/
Affected files:
M user/src/com/google/gwt/uibinder/rebind/messages/MessagesWriter.java
Index:
LGTM
On 2011/06/09 19:40:50, rjrjr wrote:
http://gwt-code-reviews.appspot.com/1454809/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM w/nitlets
http://gwt-code-reviews.appspot.com/1453810/diff/3001/dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java
File dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java (right):
SGTM, I'll double-check my settings.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10308
Author: rj...@google.com
Date: Thu Jun 9 09:49:05 2011
Log: Better error message for ui:attribute
Review at http://gwt-code-reviews.appspot.com/1454809
Review by: rchan...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10308
Modified:
Revision: 10309
Author: jlaba...@google.com
Date: Thu Jun 9 09:55:08 2011
Log: Adding Column#set/getCellStyleNames() to specify style names to
apply to individual cells of a CellTable/DataGrid. Users can set a general
style name to apply to the column, or override
committed as r10309
http://gwt-code-reviews.appspot.com/1446817/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Just gave it a quick look.
- There should be overloads with SafeUri for (at least)
from/appendTrustedNameAndValue and for/appendBackgroundImage adding
url( and ) around the SafeUri#asString() value.
- There's a TODO in ClippedImageImplSomething about using these methods
once they're
Eric, Ray, did you guys want to weigh in also?
http://gwt-code-reviews.appspot.com/1453810/diff/3001/dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java
File dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java (right):
Reviewers: jlabanca,
Description:
NOTE: At this point, I'm looking for a design review of the code while
I'm in the early stages of implementation. This isn't ready for a code
review.
Adding a new ElementBuilder API to build DOM Elements using the builder
pattern. There are two
http://gwt-code-reviews.appspot.com/1453810/diff/3001/dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java
File dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java (right):
http://gwt-code-reviews.appspot.com/1453810/diff/3001/dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java
File dev/core/src/com/google/gwt/dev/jjs/AstConstructor.java (right):
lgtm 2
http://gwt-code-reviews.appspot.com/1453810/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10310
Author: b...@google.com
Date: Thu Jun 9 14:18:11 2011
Log: Add RequestFactory annotation processor to tool directory.
Review at http://gwt-code-reviews.appspot.com/1447815
Patch by: bobv
Review by: rjrjr
My 2 cents:
The pattern for adding child elements seems a little surprising to me.
I'm used to seeing builder methods always return an object to keep the
building going. But with this, startOption() (for example) jumps on a
tangent, building something else -- with no way to return to building
the
Reviewers: tobyr, zundel,
Description:
Updated rpc generator result caching for field serializers to use type
signature instead of last modified time.
Removed GeneratorContextExt.getSourceLastModifiedTime().
Please review this at http://gwt-code-reviews.appspot.com/1446818/
Affected files:
Overall LGTM. I had to update CFA in my recent CL on class literal
optimization to treat an invocation of Object.getClass() as rescuing the
class literals of any instantiated types as well as to handle the new
Immortal CodeGenTypes.
We may have to revisit the UnifyAstVisitor, since it may need
Revision: 10312
Author: sco...@google.com
Date: Thu Jun 9 11:46:19 2011
Log: JavaAstConstructor uses UnifyAst.
http://gwt-code-reviews.appspot.com/1453810/
http://code.google.com/p/google-web-toolkit/source/detail?r=10312
Added:
On 2011/06/09 21:20:52, jhumphries wrote:
But with this, startOption() (for example) jumps on a tangent,
building
something else -- with no way to return to building the original
element.
In a previous API I liked, we actually constructed HTML entirely
vertically using methods that returned
Hmm... it seems fairly reasonable, except for the part about having to
write an enormous amount of code to cover all of HTML. (If it weren't
client-side code, I'd go with a more concise API and do more checking at
runtime.)
I wonder if we should generate the code? (Not in a GWT generator, but as
On Thu, Jun 9, 2011 at 6:13 PM, codesite-nore...@google.com wrote:
Revision: 10311
Author: b...@google.com
Date: Thu Jun 9 11:31:51 2011
Log: Support RequestContext composition (issue 6234) and mix-in (issue
6035).
Remove no-method-overloads restriction by using a full method
Reviewers: rjrjr,
Description:
Suport polymorphic return values in RequestFactory.
Issue 5367.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1453811/
Affected files:
M
http://gwt-code-reviews.appspot.com/1446818/diff/1/user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
File user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
(right):
Reviewers: zundel,
Message:
small review
Description:
Report errors cleanly and don't blow up.
Please review this at http://gwt-code-reviews.appspot.com/1451814/
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
Index:
Reviewers: jbrosenberg,
Message:
tiny review
Description:
Fixes a compile error that occurs with code like this:
interface SubIteratorE extends IteratorE {
}
class Foo implements IterableString {
@Override
public SubIteratorString iterator() {
return null;
}
}
We were trying to
http://gwt-code-reviews.appspot.com/1451814/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
File dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java (right):
http://gwt-code-reviews.appspot.com/1451814/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode665
Answers + fix for a broken test.
http://gwt-code-reviews.appspot.com/1451814/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
File dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java (right):
Do we have a unit test which exercises this (and includes regression for
the non-SubIterator case? Is this only a problem in the new
GwtAstBuilder? Isn't there a similar issue in the old GenerateJavaAST
code?
Do we have a unit test which exercises this (and includes regression for
the non-SubIterator case? Is this only a problem in the new
GwtAstBuilder? Isn't there a similar issue in the old GenerateJavaAST
code?
http://gwt-code-reviews.appspot.com/1450814/
--
http://gwt-code-reviews.appspot.com/1446818/diff/1/user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
File user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
(right):
No unit test for this, it'd be good to add one.
GenerateJavaAST already has the right code, in a slightly different
form:
try {
// TODO: This is slow! Cache lookup.
Field privateField =
ForeachStatement.class.getDeclaredField(collectionElementType);
privateField.setAccessible(true);
54 matches
Mail list logo
