[gwt-contrib] Re: More TCK tests (issue1567804)

2011-10-17 Thread Ray Ryan
Missed this, looking now.

On Mon, Oct 17, 2011 at 10:26 AM, ncha...@google.com wrote:

 On 2011/10/13 18:53:54, Nick Chalko wrote:

 ping

 http://gwt-code-reviews.**appspot.com/1567804/http://gwt-code-reviews.appspot.com/1567804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: More TCK tests (issue1567804)

2011-10-17 Thread Ray Ryan
LGTM
On Oct 17, 2011 4:26 PM, ncha...@google.com wrote:


 http://gwt-code-reviews.**appspot.com/1567804/diff/1/**
 user/test/org/hibernate/**jsr303/tck/tests/bootstrap/**customprovider/**
 TckTestValidatorFactory.javahttp://gwt-code-reviews.appspot.com/1567804/diff/1/user/test/org/hibernate/jsr303/tck/tests/bootstrap/customprovider/TckTestValidatorFactory.java
 File
 user/test/org/hibernate/**jsr303/tck/tests/bootstrap/**customprovider/**
 TckTestValidatorFactory.java
 (right):

 http://gwt-code-reviews.**appspot.com/1567804/diff/1/**
 user/test/org/hibernate/**jsr303/tck/tests/bootstrap/**customprovider/**
 TckTestValidatorFactory.java#**newcode16http://gwt-code-reviews.appspot.com/1567804/diff/1/user/test/org/hibernate/jsr303/tck/tests/bootstrap/customprovider/TckTestValidatorFactory.java#newcode16
 user/test/org/hibernate/**jsr303/tck/tests/bootstrap/**customprovider/**
 TckTestValidatorFactory.java:**16:
 package org.hibernate.jsr303.tck.**tests.bootstrap.**customprovider;
 On 2011/10/17 20:55:03, rjrjr wrote:

 Is it really appropriate for this to be in the org.hibernate space

 rather than

 com.google.gwt?


 This is the same directory as the test.   For some of the tests it was
 important to be in the same package.

 http://gwt-code-reviews.**appspot.com/1567804/http://gwt-code-reviews.appspot.com/1567804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add HasEnabled to SuggestBox (issue1567803)

2011-10-14 Thread Ray Ryan
When I run your sample I never see the suggest box. What should I be typing?

On Wed, Oct 12, 2011 at 9:10 PM, stephen.haber...@gmail.com wrote:

 I published a demo here:

 http://sh-hello.appspot.com/


 http://gwt-code-reviews.**appspot.com/1567803/http://gwt-code-reviews.appspot.com/1567803/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Generate unique class names in AbstractClientBundleGenerator at design time. (issue1565805)

2011-10-11 Thread Ray Ryan
LGTM

On Tue, Oct 11, 2011 at 11:53 AM, scheg...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 Generate unique class names in AbstractClientBundleGenerator at design
 time.


 Please review this at 
 http://gwt-code-reviews.**appspot.com/1565805/http://gwt-code-reviews.appspot.com/1565805/

 Affected files:
  M user/src/com/google/gwt/**resources/rebind/context/**
 AbstractClientBundleGenerator.**java


 Index: user/src/com/google/gwt/**resources/rebind/context/**
 AbstractClientBundleGenerator.**java
 ==**==**===
 --- user/src/com/google/gwt/**resources/rebind/context/**
 AbstractClientBundleGenerator.**java (revision 10696)
 +++ user/src/com/google/gwt/**resources/rebind/context/**
 AbstractClientBundleGenerator.**java (working copy)
 @@ -50,6 +50,7 @@
  import com.google.gwt.user.rebind.**ClassSourceFileComposerFactory**;
  import com.google.gwt.user.rebind.**SourceWriter;

 +import java.beans.Beans;
  import java.io.PrintWriter;
  import java.io.Serializable;
  import java.net.URL;
 @@ -968,6 +969,11 @@

 toReturn.append(_ + getClass().getSimpleName());

 +// If design time, generate new class each time to allow reloading.
 +if (Beans.isDesignTime()) {
 +  toReturn.append(_designTime + System.currentTimeMillis());
 +}
 +
 return toReturn.toString();
   }





-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Change the superclass of the translatable version of (issue1565803)

2011-10-06 Thread Ray Ryan
LGTM
On Oct 6, 2011 5:07 PM, skybr...@google.com wrote:
 Reviewers: rjrjr,

 Description:
 Change the superclass of the translatable version of
 junit.framework.AssertionFailedError to match the JVM version,
 for consistency when catching java.lang.AssertionError in
 testing tools.

 Fixes issue 6863.


 Please review this at http://gwt-code-reviews.appspot.com/1565803/

 Affected files:
 M

user/super/com/google/gwt/junit/translatable/junit/framework/AssertionFailedError.java


 Index:

user/super/com/google/gwt/junit/translatable/junit/framework/AssertionFailedError.java
 ===
 ---

user/super/com/google/gwt/junit/translatable/junit/framework/AssertionFailedError.java

 (revision 10689)
 +++

user/super/com/google/gwt/junit/translatable/junit/framework/AssertionFailedError.java

 (working copy)
 @@ -18,7 +18,7 @@
 /**
 * Translatable version of JUnit's codeAssertionFailedError/code.
 */
 -public class AssertionFailedError extends Error {
 +public class AssertionFailedError extends AssertionError {

 public AssertionFailedError() {
 }



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Serialization of Final Fields in RPC (issue1380807)

2011-10-04 Thread Ray Ryan
Not really practical.

On Tue, Oct 4, 2011 at 10:35 AM, stephen.haber...@gmail.com wrote:

 I meant to comment; this would all be a lot simpler if GWT-RPC could
 serialize final fields as the default/only behavior (basically remove
 any configuration variables to turn it on/off).

 I understand this isn't viable for the next release, as it's a breaking
 change, and users might be putting values in final fields that they
 don't want to go over the wire (although hopefully that is rare).

 But, just throwing it out there, would it be possible to deprecate all
 of the configuration variables, and say that in the next major release,
 final fields will always be serialized?

 Then a lot of the conditional flags and logic could be removed, and the
 implementation would be simplified. Which isn't critical, but I think
 would be nice.

 http://gwt-code-reviews.**appspot.com/1380807/http://gwt-code-reviews.appspot.com/1380807/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Do RF validation via APT instead of command line. (issue1547804)

2011-09-15 Thread Ray Ryan
Jeff, how are you launching your app under eclipse? Are you making m2e fire
gwt:run, or are you using Google Plugin for Eclipse  Run as Web App?

On Thu, Sep 15, 2011 at 2:58 AM, Jeff Larsen larse...@gmail.com wrote:

 I know I would like it if the RequestFactory stuff was pushed into maven
 central.

 Don't make gwt-servlet into a real dependency, add it to the list of
 dependencies in your plugin.


   plugin
 groupIdorg.bsc.maven/**groupId
 artifactIdmaven-processor-**plugin/artifactId
 version2.0.5/version
 executions
   execution
 idprocess/id
 goals
   goalprocess/goal
 /goals
 phasegenerate-sources/**phase
 configuration
   !-- TODO(rjrjr) Can this explicit processor line go away if

 we depend
on requestfactory-apt directly? Is that jar in maven
 central?
 --
   processors

 processorcom.google.web.**bindery.requestfactory.apt.**
 RfValidator/processor
   /processors
 /configuration
   /execution
 /executions
 dependencies
   dependency
 groupIdcom.google.gwt/groupId
artifactIdgwt-servlet/artifactId
version${gwt-version}/version
 /dependency
 /dependencies
 /plugin
   /plugins


 When you switch that to requestfactory-apt, you should be able to get rid
 of the explicit binding as well, but I have it in mine for documentation
 purposes.

 I was just working on this last week for some annotation processors I've
 written for my application to get around writing the proxies for
 RequestFactory, building the columns that map to those proxies then also
 building the constantswithlookup to map the values in there.

 I needed to do some additional steps when doing this with m2e aswell. Since
 there are no m2e connectors for any of the annotation processor maven
 plugins, I worked around that with the build-helper plugin.

 The build helper plugin allows you to specify additional source folders, so
 I just mapped a new source folder to where my maven annotation processor was
 going. I think I found a bug in build-helper-connector where you can't
 specify source folders under target, but maybe you'll have better luck.

 Then because I added a new source folder, I needed to add the maven clean
 plugin to make sure the generated artifacts were getting removed every time
 a maven clean happened.

 tl;dr

 here are the relevant bits of the pom that I was using to get all this
 working together.
 plugin
 groupIdorg.codehaus.mojo/groupId
  artifactIdbuild-helper-maven-plugin/artifactId
 version1.7/version
  executions
 execution
 idadd-source/id
  phasegenerate-sources/phase
 goals
 goaladd-source/goal
  /goals
 configuration
 sources
  source${basedir}/generated-sources/annotations/source
 /sources
 /configuration
  /execution
 /executions
 /plugin
 plugin
 artifactIdmaven-clean-plugin/artifactId
 version2.4.1/version
  configuration
 filesets
 fileset
  directorygenerated-sources/annotations/directory
 includes
 include**/*/include
  /includes
 /fileset
 /filesets
  /configuration
 /plugin
 plugin

 groupIdorg.bsc.maven/groupId
 artifactIdmaven-processor-plugin/artifactId
  executions
 execution
 idprocess/id
  goals
 goalprocess/goal
 /goals
  phasegenerate-sources/phase
 /execution
 /executions
  dependencies
 dependency
 groupIdcom.ecologic/groupId
  artifactIdrfprocessor/artifactId
 version0.0.1-SNAPSHOT/version
 /dependency
  /dependencies

 configuration
 outputDirectory${basedir}/generated-sources/annotations/outputDirectory
  processors
 processorcom.ecologic.rfprocessor.CreateDefaults/processor
 /processors
  /configuration






-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Update the checkstule path for validation sample (issue1547803)

2011-09-14 Thread Ray Ryan
LGTM

Oops. Thanks Nick. I'll need to remember to put this on the 2.4 branch.
On Sep 13, 2011 7:27 PM, ncha...@google.com wrote:
 Reviewers: rjrjr,

 Description:
 Update the checkstule path for validation sample
 Fix the @NotSupported count


 Please review this at http://gwt-code-reviews.appspot.com/1547803/

 Affected files:
 M samples/common.ant.xml
 M user/build.xml


 Index: samples/common.ant.xml
 ===
 --- samples/common.ant.xml (revision 8405)
 +++ samples/common.ant.xml (working copy)
 @@ -189,8 +189,8 @@
 target name=checkstyle description=Static analysis of source
 gwt.checkstyle outputdirectory=${sample.build}
 fileset dir=src 
 - exclude name=org/**/super/org/**/*.java/
 - exclude
 name=com/google/gwt/sample/validation*/**/ValidationMessages.java /
 + exclude name=main/java/org/**/super/org/**/*.java/
 + exclude

name=main/java/com/google/gwt/sample/validation*/**/ValidationMessages.java

 /
 /fileset
 /gwt.checkstyle
 /target
 Index: user/build.xml
 ===
 --- user/build.xml (revision 8405)
 +++ user/build.xml (working copy)
 @@ -803,13 +803,14 @@
 countfilter match=@Failing property=jsr303.marked.Failing
 init=0/
 countfilter match=@NonTckTest
 property=jsr303.marked.NonTckTest init=0/
 countfilter match=@NotSupported
 property=jsr303.marked.NotSupported init=0/
 - countfilter match=@TestNotCompatible
 property=jsr303.marked.NotSupported init=0/
 + countfilter match=@TestNotCompatible
 property=jsr303.marked.TestNotCompatible init=0/
 /filterchain
 /scan

 !-- force to zero if not set above --
 property name=jsr303.marked.Failing value=0/
 property name=jsr303.marked.NonTckTest value=0/
 + property name=jsr303.marked.NotSupported value=0/
 property name=jsr303.marked.TestNotCompatible value=0/

 echo message=Marked Failing = ${jsr303.marked.Failing} /



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-12 Thread Ray Ryan
It's resolved if Nick gives his LGTM

On Mon, Sep 12, 2011 at 9:34 AM, rchan...@google.com wrote:

 LGTM. Once the ScriptAssert issue gets resolved.

 On 2011/09/12 16:27:34, rjrjr wrote:



 http://gwt-code-reviews.**appspot.com/1537805/http://gwt-code-reviews.appspot.com/1537805/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: When -XdisableClassMetadata is used, Class.getName() can return Class$SseedNumber as a class n... (issue1540804)

2011-09-12 Thread Ray Ryan
LGTM

Nice catch. Is there really no test to extend for this? :-(

On Mon, Sep 12, 2011 at 5:39 PM, cromwell...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 When -XdisableClassMetadata is used, Class.getName() can return
 Class$SseedNumber as a class name. However, there are other modes
 where it can return Class$obfuscated function name. In some rare
 cases, these two could collide of if an obfuscated name of a class ended
 up as something like 'S123'. This patch changes the WebModeClientOracle
 to treat Class$S123 differently than 'S123' when deobfuscating class
 names.


 Please review this at 
 http://gwt-code-reviews.**appspot.com/1540804/http://gwt-code-reviews.appspot.com/1540804/

 Affected files:
  M user/src/com/google/gwt/rpc/**server/WebModeClientOracle.**java


 Index: user/src/com/google/gwt/rpc/**server/WebModeClientOracle.**java
 ==**==**===
 --- user/src/com/google/gwt/rpc/**server/WebModeClientOracle.**java
 (revision 10636)
 +++ user/src/com/google/gwt/rpc/**server/WebModeClientOracle.**java
 (working copy)
 @@ -383,12 +383,14 @@
   @Override
   public String getTypeName(String seedName) {
 // TODO: Decide how to handle the no-metadata case
 +ClassData data = null;
 if (seedName.startsWith(Class$)**) {
   seedName = seedName.substring(6);
 -}
 -ClassData data = seedNamesToClassData.get(**seedName);
 +  data = seedIdsToClassData.get(**seedName);
 +}
 +
 if (data == null) {
 -  data = seedIdsToClassData.get(**seedName);
 +  data = seedNamesToClassData.get(**seedName);
 }
 return data == null ? null : data.typeName;
   }




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-09 Thread Ray Ryan
My main problem was that I had lost the web.xml file (duh).

Still trying to figure out the ScriptAssertValidator issue. I wonder if an
extra copy is being pulled in by maven.


On Thu Sep 08 18:15:07 GMT-700 2011, Nick Chalko wrote:



  org.hibernate.validator.**constraints.impl.**ScriptAssertValidator



 So ScriptAssertValidator should not be getting compiled.  Let me try to
 find where it is excluded.


 user/src/org/hibernate/validator/HibernateValidator.gwt.xml  excludes that
 file with.

   source path=constraints

 exclude name=impl/scriptassert/ /

 exclude name=super/ /

   /source

 Not sure why this is not used.

 --
 Nick Chalko | Software Engineer | 
 ncha...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Anchalko%40google.com
  |   :-)



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-09 Thread Ray Ryan
Thanks.

Do I need the validation-api dependency (in any of samples/*/pom.xml), or
does gwt 2.4.0 bring that along?

On Fri, Sep 9, 2011 at 12:09 PM, drfibona...@google.com wrote:


 http://gwt-code-reviews.**appspot.com/1537805/diff/2002/**
 samples/validation/pom.xmlhttp://gwt-code-reviews.appspot.com/1537805/diff/2002/samples/validation/pom.xml
 File samples/validation/pom.xml (right):

 http://gwt-code-reviews.**appspot.com/1537805/diff/2002/**
 samples/validation/pom.xml#**newcode200http://gwt-code-reviews.appspot.com/1537805/diff/2002/samples/validation/pom.xml#newcode200
 samples/validation/pom.xml:**200:
 artifactIdexec-maven-plugin**/artifactId
 Looks like the whole pluginManagement section can be omitted as I don't
 see exec-maven-plugin anywhere else in the POM.


 http://gwt-code-reviews.**appspot.com/1537805/http://gwt-code-reviews.appspot.com/1537805/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-09 Thread Ray Ryan
On Thu, Sep 8, 2011 at 6:15 PM, Nick Chalko ncha...@google.com wrote:



  org.hibernate.validator.**constraints.impl.**ScriptAssertValidator



 So ScriptAssertValidator should not be getting compiled.  Let me try to
 find where it is excluded.


 user/src/org/hibernate/validator/HibernateValidator.gwt.xml  excludes that
 file with.

   source path=constraints

 exclude name=impl/scriptassert/ /

 exclude name=super/ /

   /source

 Not sure why this is not used.


It looks like the ScriptAssertValidator problem is actually on the
@ScriptAssert annotation itself:

@Target({ TYPE })
@Retention(RUNTIME)
*@Constraint(validatedBy = ScriptAssertValidator.class)*
@Documented
public @interface ScriptAssert {

The type oracle tries to process ScriptAssertValidator.class, but it can't
because of nick's excludes line. No harm is done, but the console noise is
unpleasant. And I wonder what would happen if someone actually tried to use
@ScriptAssert?

Nick, do you emulate the rest of those constraints, or do they just work?
Should I exclude the whole package?

Or perhaps I should put in a no-op super source implementation
of ScriptAssertValidator. We've done that kind of thing before.

rjrjr

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Scripts and configuration to upload GWT bits to Maven repos (issue1520810)

2011-09-09 Thread Ray Ryan
LGTM

On Fri, Sep 9, 2011 at 1:26 PM, rchan...@google.com wrote:


 http://gwt-code-reviews.**appspot.com/1520810/diff/7016/**maven/lib-gwt.shhttp://gwt-code-reviews.appspot.com/1520810/diff/7016/maven/lib-gwt.sh
 File maven/lib-gwt.sh (right):

 http://gwt-code-reviews.**appspot.com/1520810/diff/7016/**
 maven/lib-gwt.sh#newcode99http://gwt-code-reviews.appspot.com/1520810/diff/7016/maven/lib-gwt.sh#newcode99
 maven/lib-gwt.sh:99: maven-deploy-file $mavenRepoUrl $mavenRepoId
 $pomDir/gwt/pom.xml $pomDir/gwt/pom.xml
 On 2011/09/09 19:59:04, rjrjr wrote:

 This should happen between the for loops.


 Done.


 http://gwt-code-reviews.**appspot.com/1520810/http://gwt-code-reviews.appspot.com/1520810/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-09 Thread Ray Ryan
On Fri, Sep 9, 2011 at 1:15 PM, Ray Ryan rj...@google.com wrote:

 On Thu, Sep 8, 2011 at 6:15 PM, Nick Chalko ncha...@google.com wrote:



  org.hibernate.validator.**constraints.impl.**ScriptAssertValidator



 So ScriptAssertValidator should not be getting compiled.  Let me try to
 find where it is excluded.


 user/src/org/hibernate/validator/HibernateValidator.gwt.xml  excludes that
 file with.

   source path=constraints

 exclude name=impl/scriptassert/ /

 exclude name=super/ /

   /source

 Not sure why this is not used.


 It looks like the ScriptAssertValidator problem is actually on the
 @ScriptAssert annotation itself:

 @Target({ TYPE })
 @Retention(RUNTIME)
 *@Constraint(validatedBy = ScriptAssertValidator.class)*
 @Documented
 public @interface ScriptAssert {

  The type oracle tries to process ScriptAssertValidator.class, but it can't
 because of nick's excludes line. No harm is done, but the console noise is
 unpleasant. And I wonder what would happen if someone actually tried to use
 @ScriptAssert?

 Nick, do you emulate the rest of those constraints, or do they just work?
 Should I exclude the whole package?

 Or perhaps I should put in a no-op super source implementation
 of ScriptAssertValidator. We've done that kind of thing before.

 rjrjr


Hmm. You already have a super source ScriptAssertValidator, just like all
the other ones. I still don't get what's special about this one.

Deleting that exclude line doesn't change anything. Making it exclude the
entire impl packages gets rid of the error and the sample still works.
Seeing what that does to the tests. (I assume there's a reason you aren't
already doing that.)

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Converts the validation sample to build with maven. (issue1537805)

2011-09-09 Thread Ray Ryan
On Fri, Sep 9, 2011 at 1:40 PM, Ray Ryan rj...@google.com wrote:



 On Fri, Sep 9, 2011 at 1:15 PM, Ray Ryan rj...@google.com wrote:

 On Thu, Sep 8, 2011 at 6:15 PM, Nick Chalko ncha...@google.com wrote:



  org.hibernate.validator.**constraints.impl.**ScriptAssertValidator



 So ScriptAssertValidator should not be getting compiled.  Let me try to
 find where it is excluded.


 user/src/org/hibernate/validator/HibernateValidator.gwt.xml  excludes
 that file with.

   source path=constraints

 exclude name=impl/scriptassert/ /

 exclude name=super/ /

   /source

 Not sure why this is not used.


 It looks like the ScriptAssertValidator problem is actually on the
 @ScriptAssert annotation itself:

 @Target({ TYPE })
 @Retention(RUNTIME)
 *@Constraint(validatedBy = ScriptAssertValidator.class)*
 @Documented
 public @interface ScriptAssert {

  The type oracle tries to process ScriptAssertValidator.class, but it
 can't because of nick's excludes line. No harm is done, but the console
 noise is unpleasant. And I wonder what would happen if someone actually
 tried to use @ScriptAssert?

 Nick, do you emulate the rest of those constraints, or do they just work?
 Should I exclude the whole package?

 Or perhaps I should put in a no-op super source implementation
 of ScriptAssertValidator. We've done that kind of thing before.

 rjrjr


 Hmm. You already have a super source ScriptAssertValidator, just like all
 the other ones. I still don't get what's special about this one.

 Deleting that exclude line doesn't change anything. Making it exclude the
 entire impl packages gets rid of the error and the sample still works.
 Seeing what that does to the tests. (I assume there's a reason you aren't
 already doing that.)


Ah ha! It is you, *you Nick Chalko* that are making this happen! In your
super source you have a copy of ScriptAssert.java, where you have no other
annotations emulated. It includes an annotation with the reference to the
ScriptAssertValidator.class. (I suspect this is a bug in super source if you
squint just right, but I don't care.)

If I delete that file the noise goes away, the sample runs, and the tests
all pass. Any idea why it's there?

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Deprecate DeferredCommand and IncrementalCommand. (issue982802)

2011-09-08 Thread Ray Ryan
This isn't a release notes issue. The deprecation happened quite a while
ago. DevGuideCodingBasicsDelayed.html is very, very out of date.

On Thu, Sep 8, 2011 at 3:31 PM, Doug Anderson do...@google.com wrote:

 Eric:

 I can note this and any other deprecation you want in the release notes for
 next release, say 2.5  (or, if anything was deprecated for 2.4 and we missed
 it, I can also update the 2.4 notes!).

 The actual Javadoc is maintained by you folks, so any changes you want in
 that needs to be made at your end, I'm pretty sure.

 Doug



 On Thu, Sep 8, 2011 at 3:01 PM, Eric Ayers zun...@google.com wrote:


 http://google-web-toolkit.googlecode.com/svn/javadoc/latest/com/google/gwt/user/client/DeferredCommand.html

 has a pointer to the blessed alternative (Scheduler).

 @Doug, can you flag this page as needing an update?

 On Thu, Sep 8, 2011 at 4:43 PM, mark.w...@potentbyte.com wrote:

 Since this is deprecated what is the alternative to these classes?

 The documentation continues to provide these as examples.
 http://code.google.com/**webtoolkit/doc/latest/**
 DevGuideCodingBasicsDelayed.**htmlhttp://code.google.com/webtoolkit/doc/latest/DevGuideCodingBasicsDelayed.html

 On 2010/10/13 17:13:43, rjrjr wrote:

 On Wed, Oct 13, 2010 at 5:28 AM, mailto:zun...@google.com wrote:


  
  http://gwt-code-reviews.**appspot.com/982802/diff/1/5http://gwt-code-reviews.appspot.com/982802/diff/1/5
  File user/src/com/google/gwt/user/**client/DeferredCommand.java

 (right):

 
  http://gwt-code-reviews.**appspot.com/982802/diff/1/5#**newcode27http://gwt-code-reviews.appspot.com/982802/diff/1/5#newcode27
  user/src/com/google/gwt/user/**client/DeferredCommand.java:**27: *
   API prevents effective mocking.
  On 2010/10/13 03:50:36, rjrjr wrote:
 
  I thought it was deprecated because it was redundant...TMI?
 
 
  IMHO, I don't think you can give too much information on the impetus
  behind this change.  Its going to be painful to see all those
  deprecation warnings come out the first time for most users.



  We might want to provide more justification then. We're not just

 deprecating

 the thing for mockability, it's redundant code, right?


  
 
  http://gwt-code-reviews.**appspot.com/982802/showhttp://gwt-code-reviews.appspot.com/982802/show
 




 http://gwt-code-reviews.**appspot.com/982802/http://gwt-code-reviews.appspot.com/982802/




 --
 Eric Ayers | Software Engineer | zun...@google.com | +1 404 487 9229




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Event handling support for UiRenderer (issue1534806)

2011-09-07 Thread Ray Ryan
LGTM

On Wed Sep 07 11:50:58 GMT-700 2011, rchan...@google.comgt wrote:



 http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.javahttp://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java
 File
 user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java
 (right):


 http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java#newcode60
 user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java:60http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java%23newcode60%0Auser/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java:60
 :
 if (!isParentOrRenderer(parentOrRoot, RENDERED_ATTRIBUTE)) {
 On 2011/09/07 17:30:19, rjrjr wrote:
  Should call the single arg version.

 No, because UiRendererDispatcher is a static class, so it can only call
 static methods.


  This method is null tolerant. Should it NPE instead?

 Done. Added validation for null on the first three parameters, throwing
 NPEs on null receiver, root and events.

  If so, does that fix belong
  in isParentOrRenderer(Element, String)?


 No, isParentOrRenderer(Element, String) is designed to be a safe test
 and it is used elsewhere in that way. I'd find a NPE from it kind of
 surprising.


 http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java#newcode322
 user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java:322http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java%23newcode322%0Auser/src/com/google/gwt/uibinder/client/impl/AbstractUiRenderer.java:322
 :
 * Walks up the parents of the {@code rendered} element to ascertain that
 it is attached to the
 On 2011/09/07 17:30:19, rjrjr wrote:
  In DevMode, ...
  ... Always returns true in ProdMode

 Done.


 http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.javahttp://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java
 File user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java
 (right):


 http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java#newcode28
 user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java:28http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/diff/10001/user/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java%23newcode28%0Auser/src/com/google/gwt/uibinder/client/impl/UiBinderUtil.java:28
 :
 public class UiBinderUtil {
 On 2011/09/07 17:30:19, rjrjr wrote:
  As we discussed offline, it might not be the right time to make this
 move. But
  if you keep it, could you add a note to the issue tracker mentioning
 it as a
  breaking change (tag ReleaseNote=breakingChange)

 I'd rather back off. The fix in GPE is not obvious to me.

 http://gwt-code-reviews.appspot.com/1534806/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1534806/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] dependencies/qualified refs

2011-09-07 Thread Ray Ryan
Scott says:

Stephen misunderstands what Dependencies.apiRefs means (which isn't
suprising, it's not well commented).

The simple and qualified references answer the question What references do
I depend on to correctly parse and resolve the source code?

Api refs answer the question What program elements do I need to construct a
TypeOracle representation of this class?

In other words, api refs only include things like superclass, implemented
interfaces, method parameter and return types, field types, etc.  Things you
have to have to construct a valid TypeOracle representation.  Method bodies
do not contribute to this list at all.

Simple and qualified references is generally a superset of that, includes
anything references in method bodies.

On Wed, Sep 7, 2011 at 8:52 AM, Stephen Haberman stephen.haber...@gmail.com
 wrote:

 Hi,

 There was one place in the name mangling that I had to leave an ugly
 replacement:


 https://github.com/stephenh/scalagwt-gwt/blob/embed/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#L109

 allValidClasses is by internal name now, but the qualified references
 coming from ecj are by source name.

 (Hm, if these are top-level only units, then a simple replacement would
 be okay.  Does anyone know if that is the case, off the top of their
 heads? I'll fire up the debugger later and see if I can tell.)

 But the other reason I ask on the list instead of in the review is that,
 for scalagwt itself, I don't have these ecj data structures, so I'm
 inferring the simple and qualified refs from the api refs:


 https://github.com/stephenh/scalagwt-gwt/blob/embed/dev/core/src/com/google/gwt/dev/javac/Dependencies.java#L62

 I'm pretty sure this approach is okay, but my concern is that, poking
 around in the debugger, ecj, ends up with a whole lot more strings as
 simple/qualified refs than my current, albeit somewhat naive approach
 in buildFromApiRefs.

 Given I'd already written buildFromApiRefs, I was tempted to use it
 for the .java files as well, since api refs are already internal names,
 and then I could remove the source - internal mangling. But I didn't
 have enough confidence to do that just yet.

 If anyone could comment on buildFromApiRefs, both in theory and the
 current implementation, just for scalagwt but also whether it's
 potentially usable for the java side, I'd appreciate it.

 But I'll nonetheless look into whether the qualifiedReferences are
 only top-level units, in which case the name mangling should be okay.
 I didn't think of that until just now.

 - Stephen


 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Comment on RequestFactory_2_4 in google-web-toolkit

2011-09-06 Thread Ray Ryan
Fixed, thanks.

On Monday, September 5, 2011, wrote:

 Comment by ido@gmail.com:

 In Improved request batching section the sample has typo:
 ctxB.requestB().to(new ReceiverBoolean(){});
 should be

 ctxB.requestB().to(new ReceiverInteger(){});

 For more information:
 http://code.google.com/p/**google-web-toolkit/wiki/**RequestFactory_2_4http://code.google.com/p/google-web-toolkit/wiki/RequestFactory_2_4

 --
 http://groups.google.com/**group/Google-Web-Toolkit-**Contributorshttp://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Event handling support for UiRenderer (issue1534806)

2011-09-06 Thread Ray Ryan
Oh, crazy! I didn't notice that you were invoking it at compile time. And I
suppose that's going to let it fail at compile time instead of runtime,
isn't it? Okey doke.

On Tuesday, September 6, 2011, wrote:

 On 2011/09/02 00:01:48, rjrjr wrote:

 http://gwt-code-reviews.**appspot.com/1534806/diff/1/**
 user/src/com/google/gwt/**uibinder/rebind/**UiBinderWriter.java#**
 newcode1202http://gwt-code-reviews.appspot.com/1534806/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1202

 user/src/com/google/gwt/**uibinder/rebind/**UiBinderWriter.java:1202: try

 {

 Should use the TypeOracle, not reflection


 Here I need to actually call XEvent.getType().getName() to figure out
 the string the class is associated with. How do I go about using
 TypeOracle for that?


 http://gwt-code-reviews.**appspot.com/1534806/http://gwt-code-reviews.appspot.com/1534806/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Updates pom.xml to use new Request Factory libraries (issue1541803)

2011-09-06 Thread Ray Ryan
LGTM

Nice!

On Tue, Sep 6, 2011 at 1:22 PM, rchan...@google.com wrote:

 Reviewers: rjrjr, drfibonacci,

 Description:
 Updates pom.xml to use new Request Factory libraries


 Please review this at 
 http://gwt-code-reviews.**appspot.com/1541803/http://gwt-code-reviews.appspot.com/1541803/

 Affected files:
  M samples/mobilewebapp/pom.xml


 Index: samples/mobilewebapp/pom.xml
 ==**==**===
 --- samples/mobilewebapp/pom.xml(revision 10615)
 +++ samples/mobilewebapp/pom.xml(working copy)
 @@ -48,10 +48,15 @@
 !-- Google Web Toolkit (GWT) itself --

 dependency
 -  groupIdcom.google.gwt/**groupId
 -  artifactIdgwt-servlet/**artifactId
 +  groupIdcom.google.web.**bindery/groupId
 +  artifactIdrequestfactory-**server/artifactId
   version${gwtVersion}/**version
   scoperuntime/scope
 +/dependency
 +dependency
 +  groupIdcom.google.web.**bindery/groupId
 +  artifactIdrequestfactory-**apt/artifactId
 +  version${gwtVersion}/**version
 /dependency
 dependency
   groupIdcom.google.gwt/**groupId
 @@ -75,12 +80,6 @@
 /dependency

 !-- GWT RequestFactory will use JSR 303 javax.validation if you let it
 --
 -dependency
 -  groupIdjavax.validation/**groupId
 -  artifactIdvalidation-api/**artifactId
 -  version1.0.0.GA/version
 -  classifiersources/**classifier
 -/dependency
 dependency
   groupIdorg.hibernate/**groupId
   artifactIdhibernate-**validator/artifactId
 @@ -97,17 +96,7 @@
   /exclusions
 /dependency

 -!-- GWT RequestFactory requires org.json --
 -!-- TODO: can we declare the json and validation dependencies
 somewhere for the world to pick up,
 -rather than requiring everyone to know about them? --
 -dependency
 -  groupIdorg.json/groupId
 -  artifactIdjson/artifactId
 -  version20090211/version
 -/dependency
 -
 !-- Google App Engine (GAE) itself --
 -
 dependency
   groupIdcom.google.appengine**/groupId
   artifactIdappengine-api-1.0-**sdk/artifactId
 @@ -172,11 +161,11 @@
   version1.6.1/version
 /dependency
   /dependencies
 -
 +
   build
 -!-- Generate compiled stuff in the folder used for developing mode
 --
 +!-- Generate compiled stuff in the folder used for developing mode
 --
 outputDirectory${project.**build.directory}/${project.**
 build.finalName}/WEB-INF/**classes/outputDirectory
 -
 +
 plugins
   !-- GWT Maven Plugin--
   plugin
 @@ -200,7 +189,7 @@
 version${gwtVersion}/**version
   /dependency
 /dependencies
 -!-- JS is only needed in the package phase, this speeds up
 testing --
 +!-- JS is only needed in the package phase, this speeds up
 testing --
 executions
   execution
 phaseprepare-package/phase
 @@ -209,8 +198,8 @@
 /goals
   /execution
 /executions
 -!-- Plugin configuration. There are many available options,
 - see gwt-maven-plugin documentation at codehaus.org --
 +!-- Plugin configuration. There are many available options,
 + see gwt-maven-plugin documentation at codehaus.org --
 configuration
   !-- URL that should be automatically opened in the GWT shell
 (gwt:run). --
   runTargetMobileWebApp.html/**runTarget
 @@ -255,7 +244,7 @@
   useManifestOnlyJarfalse/**useManifestOnlyJar
   forkModealways/forkMode

 -  !-- Folder for generated testing stuff --
 +  !-- Folder for generated testing stuff --
   systemProperties
 property
   namegwt.args/name




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] less name mangling

2011-09-06 Thread Ray Ryan
Thanks Steve!

We chatted, and Ray C is up for reviewing this, but I'm afraid it'll be the
usual maybe-this-week drill.


On Fri Sep 02 13:06:44 GMT-700 2011, Stephen Haberman wrote:


  I'd prefer this problem to be addressed in separate CL independent
  from of our effort.

 Here is a commit with just the name mangling. If there are any
 volunteers from the GWT team to review it, I'll promote it to
 an issue.


 https://github.com/stephenh/scalagwt-gwt/commit/5fb1f9717424b8c604f9f2c84b0bc713dcb2http://www.google.com/url?sa=Dq=https://github.com/stephenh/scalagwt-gwt/commit/5fb1f9717424b8c604f9f2c84b0bc713dcb2

 I do have a dangling todo in there, but it's somewhat
 tangential, so I'll avoid it until prompted.

 - Stephen

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Convert DynaTableRF to use maven. Clean up the docs in the other two (issue1537803)

2011-09-02 Thread Ray Ryan
ping, ready for review

On Thu Sep 01 15:08:29 GMT-700 2011, rj...@google.comgt wrote:

 http://gwt-code-reviews.appspot.com/1537803/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1537803/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] [ANN] Scala+GWT 0.1-M1 released

2011-09-01 Thread Ray Ryan
Woo hoo!

On Thu Sep 01 09:26:47 GMT-700 2011, Grzegorz Kossakowski wrote:

 Hello,

 I'm excited to announce first milestone of Scala+GWT project.

 Download (and then follow README instructions) from here:
 http://goo.gl/Ym3xUhttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgoo.gl%2FYm3xU

 Release notes (included below) can be found here: 
 http://goo.gl/H8sanhttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgoo.gl%2FH8san



 Scala+GWT 0.1-M1
 

 The first official milestone release from the Scala+GWT project.

 This release consists of just samples packaged along with snapshot
 of jars needed to build them. Those jars include our own version
 of both GWT and Scala.

 This release doesn't come with any official artifacts apart from the
 tarball you can download from 
 http://github.com/scalagwt/scalagwt-samplehttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgithub.com%2Fscalagwt%2Fscalagwt-sample
 .

 ### What works

 Samples show it the best. Here are some highlights:

   * Mixed Scala/Java projects work very well (for GWT-supported Java
 subset)
   * Most of Scala language constructs are supported
   * Most of Scala library code that makes sense in a browser context is
 supported, including Scala collections

 It's fair to say that we are not sure how far one can go with this release.
 It might be that you can already
 build something useful with what we already have. The only way to be sure
 is start hacking!

 ### Known issues

   * Compilation is very, very slow.
   * `scala.immutable.{TreeMap, TreeSet}` are not supported due to various
 bugs (thus sorted collections don't work)
   * many patterns in pattern matching logic are not supported (tough issue)
 examples include
 * pattern alternatives (`|`)
 * guard patterns (`if` guard)
   * GWT's development mode is broken
   * We are compiling with all optimizations turned off. This results in a
 slow and very large JavaScript code.

 ### Reporting issues

 We appreciate feedback. If you find something that doesn't work (e.g.
 crashes either Scala or GWT compiler)
 or JavaScript gives you weird results we'd love to hear about it. The most
 effective way of reporting issues
 is to modify `Hello World` sample to show your problem. Exact steps are:

   1. Fork `scalagwt-sample` repo from here:
 http://github.com/scalagwt/scalagwt-samplehttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgithub.com%2Fscalagwt%2Fscalagwt-sample
   2. Clone it: `git clone git://
 github.com/YOUR_USER_NAME/scalagwt-sample.git`http://www.google.com/url?sa=Dq=http%3A%2F%2Fgithub.com%2FYOUR_USER_NAME%2Fscalagwt-sample.git
   3. Modify hello world sample located in
 `src/com/google/gwt/sample/jribble/client`
   4. Commit and publish your example.
   5. File a ticket here: 
 http://github.com/scalagwt/scalagwt-samplehttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgithub.com%2Fscalagwt%2Fscalagwt-sample
  and
 mention your
  fork while explaining your issue.

 If you want to discuss your problem before reporting it, join
 [scala...@googlegroups.comhttp://www.google.com/url?sa=Dq=mailto%3Ascalagwt%40googlegroups.com
 ](http://groups.google.com/group/scalagwt).

 ### What if I don't know GWT?

 That shouldn't be a big problem. You've got Scala source code for samples
 that show basic functionality
 and provides basic setup. You may want to start with channging hello world
 sample, recompiling it and
 testing in a browser.

 ### What if I don't know Scala?

 You might still want to check out samples to see how they might look like
 in other language than Java.
 We'll be cutting a lot more of boilerplate code once GWT libraries and APIs
 receive enough of Scala's
 [pimp-love](http://www.artima.com/weblogs/viewpost.jsp?thread=179766http://www.google.com/url?sa=Dq=http%3A%2F%2Fwww.artima.com%2Fweblogs%2Fviewpost.jsp%3Fthread%3D179766
 ).

 ### Need help? Want to discuss something?

 Join us here: 
 [scala...@googlegroups.comhttp://www.google.com/url?sa=Dq=mailto%3Ascalagwt%40googlegroups.com
 ](http://groups.google.com/group/scalagwt).


 Happy playing!

 --
 Grzegorz Kossakowski

  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fixes for MobileWebApp pom.xml (issue1520809)

2011-08-31 Thread Ray Ryan
LGTM

On Wed, Aug 31, 2011 at 1:18 PM, drfibona...@google.com wrote:

 LGTM


 http://gwt-code-reviews.**appspot.com/1520809/http://gwt-code-reviews.appspot.com/1520809/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make UmbrellaException a bit more convenient to read, and test it. (issue1532803)

2011-08-26 Thread Ray Ryan
r10584


On Thu Aug 25 23:08:59 GMT-700 2011, cromwell...@google.comgt wrote:

 lgtm

 http://gwt-code-reviews.appspot.com/1532803/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1532803/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] maven source jars

2011-08-25 Thread Ray Ryan
Stephen, if you're game to do the research and the work, we're certainly
happy to have it done. Thanks either way.

On Wed Aug 24 12:39:51 GMT-700 2011, Stephen Haberman wrote:


  Hm, perhaps a few. I saw instructions on patching emma; nothing else
  is leaping out at me.

 Okay, there are patch files for json and streamhtmlparser. The latter is
 rebased. Leaving anything that is rebased or patched in the jar seems
 fine to me, since it's only a few.

 (I'm replying to myself, but I sent my previous message from the
 wrong email address, so it is probably moderated/dropped. Here's
 the text in case my previous message never shows up:)

 Taking gwt-dev-nodeps further, it looks like most of the deps come from
 bundling tomcat and htmlunit. Neither of which I use. I think it would
 be fairly easily, at least from a packaging perspective, to make
 gwt-dev-nodeps, gwt-tomcat, and gwt-htmlunit jars, none of which have
 their dependencies in them, but instead pulled in however projects
 otherwise manage transitive dependencies (poms/what have you).

 If I got a gwt-dev-nodeps/gwt-tomcat/gwt-htmlunit build working, would
 anyone be interested in that?

 - Stephen

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make UmbrellaException a bit more convenient to read, and test it. (issue1532803)

2011-08-25 Thread Ray Ryan
How's that?

On Thu Aug 25 17:04:18 GMT-700 2011, cromwell...@google.comgt wrote:



 http://gwt-code-reviews.appspot.com/1532803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.javahttp://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1532803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java
 File user/src/com/google/web/bindery/event/shared/UmbrellaException.java
 (right):


 http://gwt-code-reviews.appspot.com/1532803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java#newcode53
 user/src/com/google/web/bindery/event/shared/UmbrellaException.java:53http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1532803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java%23newcode53%0Auser/src/com/google/web/bindery/event/shared/UmbrellaException.java:53
 :
 return causes.size() == 1 ? ONE + message : MULTIPLE + message;
 Couldn't we tack on the rest of the exception messages here when there's
 multiple instead of just the first?

 http://gwt-code-reviews.appspot.com/1532803/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1532803/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Changes setters/clearers com.google.gwt.dom.client.Style to return Style instead of void (issue1530803)

2011-08-22 Thread Ray Ryan
No good. This defeats compiler optimizations big time. We don't like void
returns either, but we like big fat slow apps even less.
On Aug 22, 2011 7:50 PM, larse...@gmail.com wrote:
 Reviewers: jlabanca, rjrjr,

 Description:
 Changes Style to return itself instead of returning void.

 issue 6717

 http://code.google.com/p/google-web-toolkit/issues/detail?id=6717

 Please review this at http://gwt-code-reviews.appspot.com/1530803/

 Affected files:
 user/src/com/google/gwt/dom/client/Style.java



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Updates MobileWebApp to use GWT Maven Plugin 2.3.0-1 (issue1525806)

2011-08-18 Thread Ray Ryan
LGTM

D'oh! I thought that was meant to be *our* version string. Sorry.

On Thu Aug 18 08:55:54 GMT-700 2011, Rodrigo Chandia wrote:

 According to the GWT maven plugin web page 2.3.0-1 is the latest version.

 http://mojo.codehaus.org/gwt-maven-plugin/http://www.google.com/url?sa=Dq=http%3A%2F%2Fmojo.codehaus.org%2Fgwt-maven-plugin%2F

 El 18 de agosto de 2011 11:53, 
 rchan...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Archandia%40google.com
  escribió:


 http://gwt-code-reviews.**appspot.com/1525806/diff/1/**
 samples/mobilewebapp/pom.xmlhttp://www.google.com/url?sa=Dq=http%3A%2F%2Fgwt-code-reviews.appspot.com%2F1525806%2Fdiff%2F1%2Fsamples%2Fmobilewebapp%2Fpom.xml
 File samples/mobilewebapp/pom.xml (right):

 http://gwt-code-reviews.**appspot.com/1525806/diff/1/**
 samples/mobilewebapp/pom.xml#**newcode191http://www.google.com/url?sa=Dq=http%3A%2F%2Fgwt-code-reviews.appspot.com%2F1525806%2Fdiff%2F1%2Fsamples%2Fmobilewebapp%2Fpom.xml%23newcode191
 samples/mobilewebapp/pom.xml:**191: version2.3.0-1/version
 On 2011/08/18 15:34:16, rjrjr wrote:

 What's the -1 for? Also, tab character in this line.

 Done, untabified the file. The -1 is part of the version string
 2.3.0-1


 http://gwt-code-reviews.**appspot.com/1525806/http://www.google.com/url?sa=Dq=http%3A%2F%2Fgwt-code-reviews.appspot.com%2F1525806%2F




 --
 Rodrigo Chandia | Software Engineer | 
 rchan...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Archandia%40google.com
  | 678 7431725



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix http://code.google.com/p/google-web-toolkit/issues/detail?id=6161 (issue1520807)

2011-08-18 Thread Ray Ryan
No, but I'm not sure it can be. The error is actually legitimate: you have
@UiField Label and you're trying to stick an instance of Element in it, in
this case a funny looking one: g:Label...

My alternative is to make this error fatal, which means that you could not
use binder to render prefixed dom elements. Not a very compelling use case,
I agree, but who knows how many existing templates are already making this
error. I'm not really in the mood to be hunted down by the people I'd
break...



On Thu Aug 18 09:59:22 GMT-700 2011, mros...@google.comgt wrote:



 http://gwt-code-reviews.appspot.com/1520807/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.javahttp://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1520807/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
 File
 user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java
 (right):


 http://gwt-code-reviews.appspot.com/1520807/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java#newcode104
 user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java:104http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1520807/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java%23newcode104%0Auser/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java:104
 :
 writer.warn(elem, Prefix \%s:\ has unrecognized xmlns \%s\, 
 Will this warning be shown in proximity to where an ERROR is shown when
 it does fail?

 http://gwt-code-reviews.appspot.com/1520807/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1520807/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding a new CellTableHeaderBuilder API, which allows custom headers and footers in CellTable. C... (issue1499808)

2011-08-17 Thread Ray Ryan
LGTM

On Wed, Aug 17, 2011 at 7:32 AM, jlaba...@google.com wrote:

 A patch is coming that modifies the TableBuilder API to make it more
 flexible.  TableBuilder.Utility is going away completely, and the event
 handling implementation is moved from AbstractCellTable into
 AbstractTableBuilder.  This will allow users to implement a simple grid
 based TableBuilder with faster rendering and event handling logic.

 The new API is also much more builder-like, and simpler to boot.Once
 those changes are in, I'll revisit HeaderCreator and apply similar
 changes, hopefully eliminating the Helper class.  Either way, there will
 be follow-up changes that
 unify the terminology used in TableBuilder/HeaderCreator.



 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.javahttp://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java
 File
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java
 (right):

 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode79http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode79
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:79:
 * Example file.
 On 2011/08/16 02:16:25, skybrian wrote:

 Defines a custom table that displays a contact in each row.
 This is an example that shows how to completely customize the

 appearance of the

 headers, data rows, and footers in a CellTable.


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode143http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode143
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:143:
 * A custom header builder.
 On 2011/08/16 02:16:25, skybrian wrote:

 Renders custom headers that ...


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode164http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode164
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:164:
 public void buildHeader(Helper**ContactInfo utility) {
 On 2011/08/16 02:16:25, skybrian wrote:

 s/utility/helper/


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode209http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode209
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:209:
 * Build a single header.
 On 2011/08/16 02:16:25, skybrian wrote:

 Renders the header of one column, with the given options.


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode211http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode211
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:211:
 * @param utility the utility used to builder the header
 On 2011/08/16 02:16:25, skybrian wrote:

 used to build


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 showcase/client/content/cell/**CwCustomDataGrid.java#**newcode213http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode213
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:213:
 * @param header the header to buil
 On 2011/08/16 02:16:25, skybrian wrote:

 the Header to render


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/**
 14004/samples/showcase/src/**com/google/gwt/sample/**
 

[gwt-contrib] Re: Provides an integration test for IsRenderable (issue1527804)

2011-08-17 Thread Ray Ryan
I like #1 too. I think we should try to narrow the visibility of
PotentialElement as much as we can.

So #1 means two things , right?

• Widgets are seated in their @UiFields immediately
• In an IsRenderable owner, Element and subclasses are only available via
LazyDomElement, and @UiField Element is a compile time error

I've tweaked the test a bit (will update soon), and I'm happy to report that
composites around non-IsRenderables work as expected, with element fields
filled immediately. Given that I don't think we need to delay the switch to
using lazy widget builder by default.


On Wed Aug 17 06:14:52 GMT-700 2011, Hermes Freitas wrote:

 WidgetInterpreter and WidgetPlaceholderInterpreter shouldn't output
 LazyDomElement. Rafa, do you remember why? I don't think this aggregates any
 performance gain for us,  am I missing something?

 And I vote for #1

 On Tue, Aug 16, 2011 at 10:10 PM, Rafael Castro 
 rdcas...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Ardcastro%40google.com
  wrote:

 +hermes

 Good point, this is really tricky. The problem here is that we don't
 actually have the DOM element until the widget is attached. I see 2 options:
 1-) We force the UiField to be a LazyDomElement, so this is explicit.
 2-) We use PotentialElement with a resolver that throws an Exception (i.e.,
 it's only really resolved when it's attached).

 what do you think?

 ps.: really nice tests, thanks for putting them together!

 On Tue, Aug 16, 2011 at 5:13 PM, 
 rj...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Arjrjr%40google.com
  wrote:

 On 2011/08/17 00:12:24, rjrjr wrote:

 Ready for review.

 Rafa, this turned up one issue that concerns me: most @UiField fields
 are not filled in until the widget is attached to the dom, but we're not
 consistent about it. See the big comment in testDeep.


 http://gwt-code-reviews.**appspot.com/1527804/http://www.google.com/url?sa=Dq=http%3A%2F%2Fgwt-code-reviews.appspot.com%2F1527804%2F





 --
 --Hermes Freitas


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Provides an integration test for IsRenderable (issue1527804)

2011-08-17 Thread Ray Ryan
Cool, I've filed
http://code.google.com/p/google-web-toolkit/issues/detail?id=6701.

Can I get an LGTM and submit this thing?


On Wed Aug 17 09:58:03 GMT-700 2011, Rafael Castro wrote:

 Awesome, I like #1 too. I was driving to work this morning and thinking
 about it: #2 actually encourages bad behavior, because it'll seem it's OK to
 fiddle with the elements between calling bind and attaching, and it's really
 not. We _could_ make an effort to make it work, but it's much better to make
 the flow clearer this way: if you're using lazy widgets, your elements have
 to be lazy too.

 On Wed, Aug 17, 2011 at 9:41 AM, Ray Ryan 
 rj...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Arjrjr%40google.com
  wrote:

 I like #1 too. I think we should try to narrow the visibility of
 PotentialElement as much as we can.

 So #1 means two things , right?

 • Widgets are seated in their @UiFields immediately
 • In an IsRenderable owner, Element and subclasses are only available via
 LazyDomElement, and @UiField Element is a compile time error

 I've tweaked the test a bit (will update soon), and I'm happy to report
 that composites around non-IsRenderables work as expected, with element
 fields filled immediately. Given that I don't think we need to delay the
 switch to using lazy widget builder by default.


 On Wed Aug 17 06:14:52 GMT-700 2011, Hermes Freitas wrote:

 WidgetInterpreter and WidgetPlaceholderInterpreter shouldn't output
 LazyDomElement. Rafa, do you remember why? I don't think this aggregates any
 performance gain for us,  am I missing something?

 And I vote for #1

 On Tue, Aug 16, 2011 at 10:10 PM, Rafael Castro 
 rdcas...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Ardcastro%40google.com
  wrote:

 +hermes

 Good point, this is really tricky. The problem here is that we don't
 actually have the DOM element until the widget is attached. I see 2 options:
 1-) We force the UiField to be a LazyDomElement, so this is explicit.
 2-) We use PotentialElement with a resolver that throws an Exception (i.e.,
 it's only really resolved when it's attached).

 what do you think?

 ps.: really nice tests, thanks for putting them together!

 On Tue, Aug 16, 2011 at 5:13 PM, 
 rj...@google.comhttp://www.google.com/url?sa=Dq=mailto%3Arjrjr%40google.com
  wrote:

 On 2011/08/17 00:12:24, rjrjr wrote:

 Ready for review.

 Rafa, this turned up one issue that concerns me: most @UiField fields
 are not filled in until the widget is attached to the dom, but we're not
 consistent about it. See the big comment in testDeep.


 http://gwt-code-reviews.**appspot.com/1527804/http://www.google.com/url?sa=Dq=http%3A%2F%2Fgwt-code-reviews.appspot.com%2F1527804%2F





 --
 --Hermes Freitas




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: Re: [gwt-contrib] Aw: Regression: instanceof compiler issue

2011-08-17 Thread Ray Ryan
That's a generous offer, but I don't know that anyone is going to be able to
take you up on it. We'll keep an eye out for it here. And if you do manage
to trim your failing code down to something you can share, we'll jump.

Are you building against trunk? If not, you might try and see if the problem
is still there.

On Wed, Aug 17, 2011 at 4:22 AM, dflorey daniel.flo...@gmail.com wrote:

 Hi,
 unfortunately I did not manage to reproduce the issue in a clean project.
 But I'd like to show the issue to someone with my existing projects.
 I've been able to find a workaround by adding the module classes to a fake
 rpc service.

 I could setup a TeamViewer session to show the issue on my machine.

 Daniel

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Aw: Regression: instanceof compiler issue

2011-08-09 Thread Ray Ryan
Can you share some actual code?
On Aug 9, 2011 7:37 AM, dflorey daniel.flo...@gmail.com wrote:
 BTW: It works find in dev mode, just fails when compiled

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Aw: Regression: instanceof compiler issue

2011-08-09 Thread Ray Ryan
Anything that we can compile that actually reproduces the problem.

On Tue, Aug 9, 2011 at 8:46 AM, Daniel Florey daniel.flo...@gmail.comwrote:

 Just found out that the problem only occurs when the interface is empty
 (marker interface). As soon as any method is added, the instanceof works.
 What code snippets do you need? My project is too big to share ;-)

 Daniel


 2011/8/9 Ray Ryan rj...@google.com

 Can you share some actual code?
 On Aug 9, 2011 7:37 AM, dflorey daniel.flo...@gmail.com wrote:
  BTW: It works find in dev mode, just fails when compiled
 
  --
  http://groups.google.com/group/Google-Web-Toolkit-Contributors

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding a new CellTableHeaderBuilder API, which allows custom headers and footers in CellTable. C... (issue1499808)

2011-08-08 Thread Ray Ryan
SGTM

On Mon, Aug 8, 2011 at 7:15 PM, jlaba...@google.com wrote:

 Ray and I hashed out the names a bit.  I don't mind shortening
 CellTableHeaderCreator to HeaderCreator.  I don't think it'll cause any
 conflicts down the road.

 @rjrjr - any objections to the shorter name?



 http://gwt-code-reviews.**appspot.com/1499808/diff/1/**
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.javahttp://gwt-code-reviews.appspot.com/1499808/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java
 File
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java
 (right):

 http://gwt-code-reviews.**appspot.com/1499808/diff/1/**
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java#**newcode82http://gwt-code-reviews.appspot.com/1499808/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode82
 samples/showcase/src/com/**google/gwt/sample/showcase/**
 client/content/cell/**CwCustomDataGrid.java:82:
 public class CwCustomDataGrid extends ContentWidget {
 On 2011/08/02 03:47:57, skybrian wrote:

 This example doesn't really hang together. On the one hand it's

 showing friends

 and birthdays (like a social app or a contacts database) and on the

 other it's

 showing who's eligible for retirement benefits (like an HR app). It

 seems like

 we should pick one or the other? Perhaps take out retirement

 benefits and

 replace that with something that makes more sense for a contacts

 database?

 I removed the retirement benefits row completely.  We only need one
 example of a spanning row, so the birthday this month is fine.



  I also wonder if we should simplify it or split into two or more

 examples where

 each demonstrates one thing. For someone reading documentation, 800

 lines of

 sample code seems really big and I'd much prefer to read a smaller

 example.
 This is a sample that is meant to illustrate what you can do with GWT.
 We show source code for people who want to delve into it, but the
 primary purpose of Showcase is to show off GWT, not to provide example
 code.


 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**AbstractCellTable.javahttp://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
 File user/src/com/google/gwt/user/**cellview/client/**
 AbstractCellTable.java
 (right):

 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**AbstractCellTable.java#**
 newcode493http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java#newcode493
 user/src/com/google/gwt/user/**cellview/client/**
 AbstractCellTable.java:493:
 rawHtml = rawHtml.substring(7, rawHtml.length() - 8);
 On 2011/08/02 03:47:57, skybrian wrote:

 Perhaps add: assert tag.size() == 5


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.javahttp://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java
 File
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java
 (right):

 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java#**newcode32http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode32
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java:**32:
 public interface CellTableHeaderBuilderT {
 On 2011/08/02 03:47:57, skybrian wrote:

 How about just HeaderBuilder?


 Its specific to CellTable/DataGrid.  Other CellWidgets might have a
 different header builder.  Ray and I came up with
 CellTableHeaderCreator.


 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java#**newcode35http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode35
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java:**35:
 * A utility for building the header.
 On 2011/08/02 03:47:57, skybrian wrote:

 (or footer)


 Done.


 http://gwt-code-reviews.**appspot.com/1499808/diff/6001/**
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java#**newcode44http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode44
 user/src/com/google/gwt/user/**cellview/client/**
 CellTableHeaderBuilder.java:**44:
 public abstract class UtilityT {
 On 2011/08/02 

[gwt-contrib] Re: Fixing a bug in HasDataPresenter where paging to a negative row index causes an IndexOutOfRange ... (issue1513804)

2011-08-08 Thread Ray Ryan
LGTM

On Mon, Aug 8, 2011 at 4:42 PM, jlaba...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 Fixing a bug in HasDataPresenter where paging to a negative row index
 causes an IndexOutOfRange exception. We now properly trim the keyboard
 selected row to a non-negative value.

 Issue: 6383


 Please review this at 
 http://gwt-code-reviews.**appspot.com/1513804/http://gwt-code-reviews.appspot.com/1513804/

 Affected files:
  M user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java
  M user/test/com/google/gwt/user/**cellview/client/**
 HasDataPresenterTest.java


 Index: user/src/com/google/gwt/user/**cellview/client/**
 HasDataPresenter.java
 ==**==**===
 --- user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java
  (revision 10507)
 +++ user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java
  (working copy)
 @@ -719,8 +719,9 @@
 } else if (KeyboardPagingPolicy.CHANGE_**PAGE == keyboardPagingPolicy)
 {
   // Go to previous page.
   while (index  0) {
 -newPageStart -= pageSize;
 -index += pageSize;
 +int shift = Math.min(pageSize, newPageStart);
 +newPageStart -= shift;
 +index += shift;
   }

   // Go to next page.
 @@ -731,9 +732,10 @@
 } else if (KeyboardPagingPolicy.**INCREASE_RANGE ==
 keyboardPagingPolicy) {
   // Increase range at the beginning.
   while (index  0) {
 -newPageSize += PAGE_INCREMENT;
 -newPageStart -= PAGE_INCREMENT;
 -index += PAGE_INCREMENT;
 +int shift = Math.min(PAGE_INCREMENT, newPageStart);
 +newPageSize += shift;
 +newPageStart -= shift;
 +index += shift;
   }
   if (newPageStart  0) {
 index += newPageStart;
 Index: user/test/com/google/gwt/user/**cellview/client/**
 HasDataPresenterTest.java
 ==**==**===
 --- 
 user/test/com/google/gwt/user/**cellview/client/**HasDataPresenterTest.java
 (revision 10507)
 +++ 
 user/test/com/google/gwt/user/**cellview/client/**HasDataPresenterTest.java
 (working copy)
 @@ -750,6 +750,18 @@
 view.**assertKeyboardSelectedRowEmpty**();
 assertEquals(10, presenter.getVisibleRange().**getStart());
 assertEquals(10, presenter.getVisibleRange().**getLength());
 +
 +// Negative index out of range.
 +presenter.setVisibleRange(new Range(3, 10));
 +presenter.**setKeyboardSelectedRow(3, false, false);
 +populatePresenter(presenter);
 +presenter.flush();
 +presenter.**setKeyboardSelectedRow(-4, false, false);
 +populatePresenter(presenter);
 +presenter.flush();
 +assertEquals(0, presenter.**getKeyboardSelectedRow());
 +assertEquals(0, presenter.getVisibleRange().**getStart());
 +assertEquals(10, presenter.getVisibleRange().**getLength());
   }

   public void testSetKeyboardSelectedRowCurr**entPage() {
 @@ -880,6 +892,18 @@
 assertEquals(0, presenter.getVisibleRange().**getStart());
 pageSize += 10;
 assertEquals(pageSize, presenter.getVisibleRange().**getLength());
 +
 +// Negative index out of range.
 +presenter.setVisibleRange(new Range(3, 10));
 +presenter.**setKeyboardSelectedRow(3, false, false);
 +populatePresenter(presenter);
 +presenter.flush();
 +presenter.**setKeyboardSelectedRow(-4, false, false);
 +populatePresenter(presenter);
 +presenter.flush();
 +assertEquals(0, presenter.**getKeyboardSelectedRow());
 +assertEquals(0, presenter.getVisibleRange().**getStart());
 +assertEquals(13, presenter.getVisibleRange().**getLength());
   }

   public void testSetRowCount() {




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding a spot for database column name in Column. Allows us to create an updated SQL statement w... (issue1503806)

2011-08-04 Thread Ray Ryan
You don't have to add a param type if the app data slot is simply of type
object. No one will die if they have to cast the thing. But if the people
who originally requested this feature are willing to back off and maintain a
map instead, is even that much api clutter really warranted?

@portersi, what stopped you from using a custom column class instead of a
map?

On Tue, Aug 2, 2011 at 1:53 PM, jlaba...@google.com wrote:

 This came up before, and here is the rationale.

 If you add an app specific data object to Column, you have to add
 another parameterized type to Column.  That's more verbose, somewhat
 confusing, and it will be Void for most users anyway.  I'm a big fan of
 parameterized types because they make the API feels more bulletproof,
 but I also think parameterized types can be cumbersome if they aren't
 intended to support the most common use cases.

 Subclassing Column is good option if you need a lot of app specific
 data.  Yes you have to cast it, but who cares if you're in control of
 which columns go into your table.  In the original design, we envisioned
 people subclassing Column for app specific data.

 So, now we're special casing the data store field as a string. But MANY
 databases use String names to describe columns, and its just so
 convenient to have that field accessible from the Column itself. I've
 seen more than a few requests for this feature.  If you are using an SQL
 database, this will probably come in handy.  If you don't need it, don't
 use it, and the compiler should compile it out.


 On 2011/08/02 20:13:12, rjrjr wrote:

 It's John L's call, but that's certainly my preference.


  On Tue, Aug 2, 2011 at 1:10 PM, John Porter Simons

 porte...@google.comwrote:

   The other way we (me and dramos@) discussed doing this was, in our
  CellTable subclass, have a map from Column to String to store these

 database

  column names. I can update to that and revert this if you like.
 
 
  On Tue, Aug 2, 2011 at 1:01 PM, Ray Ryan mailto:rj...@google.com

 wrote:

 
  I was biting my tongue on this one, but I guess I'll jump in and

 agree,

  this smells bad.
 
  @jlabanca, is there no hook in Column or maybe Cell.Context where

 this

  kind of app-specific data can be added?
 
  On Tue, Aug 2, 2011 at 12:32 PM, Jeff Larsen

 mailto:larse...@gmail.com wrote:

 
   I'm inclined to agree with Stephen here. No where else in GWT

 widgetry

  is there a reference to database related things. I don't think

 this is a big

  deal either, but it seems like a more application specific thing

 rather than

  something that belongs inside GWT proper.
 
  --
  http://groups.google.com/**group/Google-Web-Toolkit-**Contributorshttp://groups.google.com/group/Google-Web-Toolkit-Contributors
 
 
 
 




 http://gwt-code-reviews.**appspot.com/1503806/http://gwt-code-reviews.appspot.com/1503806/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: GWT Code Review planned outage Thursday 4 Aug 2011 5pm ET

2011-08-04 Thread Ray Ryan
http://gwt-code-reviews.appspot.com/ will be read only in five minutes or
so.

On Wed, Aug 3, 2011 at 12:09 PM, Ray Ryan rj...@google.com wrote:

 We will be putting the Rietveld server at
 http://gwt-code-reviews.appspot.com/ into read-only mode for a few hours
 at about 5pm Eastern Time tomorrow, Thursday August 4th, to deal with some
 maintenance.

 rjrjr



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] GWT Code Review [Rietveld]: Better, faster stronger

2011-08-04 Thread Ray Ryan
In the last couple of hours we migrated
http://gwt-code-reviews.appspot.com/to a High
Replication 
Datastorehttp://googleappengine.blogspot.com/2011/01/announcing-high-replication-datastore.html.
This should be transparent to you, except for much reduced latency.[1]

Huge thanks to Fred Sauer, who did all the work.

Enjoy!

rjrjr

[1] Actually, users of the chat notification feature will need to reset that
preference to make it go again. So far this is the only hiccup we've
noticed.

   - visit http://gwt-code-reviews.appspot.com/settings
   - uncheck Notify by chat, save settings
   - recheck Notify by chat, save settings
   - accept the new chat invite from the new app id

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] GWT Code Review planned outage Thursday 4 Aug 2011 5pm ET

2011-08-03 Thread Ray Ryan
We will be putting the Rietveld server at
http://gwt-code-reviews.appspot.com/ into read-only mode for a few hours at
about 5pm Eastern Time tomorrow, Thursday August 4th, to deal with some
maintenance.

rjrjr

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make generator result caching framework api available publically. (issue1468804)

2011-08-03 Thread Ray Ryan
I'm still not crazy about having addClientData() and getClientData() on
separate objects. It seems to me that you've violated your own principal
that the GeneratorContext should be the only object that has to get passed
to the generator's helpers.

Can addClientData() move to the context? That would require getClientData()
to be able to retrieve both cached and freshly added data. Is that
practical? Sounds like it would be hugely convenient.

On Wed, Aug 3, 2011 at 12:43 PM, jbrosenb...@google.com wrote:

 Some responses.  I did consider most of your suggestions, and in fact
 for a while I did have things as you suggest, so great minds think alike
 :).  I'll outline my reasoning:

 First, IncrementalGenerators have to live in the same ecosystem as all
 the currently extent non-incremental Generator implementations.  Having
 a uniform rebind process whereby the StandardRebindOracle is agnostic to
 this greatly simplified things.  If we were starting from scratch, I
 think it would probably look quite a bit different.

 Standard Generators only return a type-name (which can possibly be
 null).  The RebindResult allows legacy generators to be cleanly wrapped
 and presented to the rebind oracle in a uniform way.

 A RebindResult is not the same as a CachedRebindResult.  The
 StandardRebindOracle uses the info in the RebindResult to decide how to
 proceed.  It may or may not choose to construct a cache entry, and that
 cached entry might contain a partial subset of newly generated types and
 types from a previous cache entry.  Since a generator itself shouldn't
 need to know how to integrate cached types and artifacts, this logic is
 better left up to the StandardRebindOracle.  The RebindResult also
 contains a target type name, and client data, so it's just a container
 to wrap a generators results.

 In the case that an IncrementalGenerator decides it can return quickly
 and request that all previously cached output can be reused, it doesn't
 need to do anything, including not re-committing cached compilation
 units and artifacts to the context, etc.  That's all magic handled by
 the rebind oracle.

 A CachedRebindResult is specifically input to an IncrementalGenerator,
 but it's cumbersome to pass it in as a separate arg to
 generateIncrementally.  Having it live in the GeneratorContext makes one
 less thing a generator implementation needs to pass around to it's
 internal methods (since GeneratorContext at the least is commonly needed
 all over the place for a generator anyway).

 Remember too, that much of the time, an IncrementalGenerator will run
 when generatorResultCaching is not enabled, such as currently for all
 web-mode compiles, so it's more of a contextual thing anyway, which
 makes it more intuitive for it to live in the GeneratorContext.

 I did have a separate GeneratContextExt interface, which extended the
 base GeneratorContext interface, but ultimately this became unnecessary,
 and it greatly simplified the framework by unifying things.

 I think you had a couple other comments offline (but not summarized
 above), I'll wait for your further comments to respond to those.


 http://gwt-code-reviews.**appspot.com/1468804/http://gwt-code-reviews.appspot.com/1468804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make generator result caching framework api available publically. (issue1468804)

2011-08-03 Thread Ray Ryan
Oh, and putClientData seems like a better name.

On Wed, Aug 3, 2011 at 1:02 PM, Ray Ryan rj...@google.com wrote:

 I'm still not crazy about having addClientData() and getClientData() on
 separate objects. It seems to me that you've violated your own principal
 that the GeneratorContext should be the only object that has to get passed
 to the generator's helpers.

 Can addClientData() move to the context? That would require getClientData()
 to be able to retrieve both cached and freshly added data. Is that
 practical? Sounds like it would be hugely convenient.


 On Wed, Aug 3, 2011 at 12:43 PM, jbrosenb...@google.com wrote:

 Some responses.  I did consider most of your suggestions, and in fact
 for a while I did have things as you suggest, so great minds think alike
 :).  I'll outline my reasoning:

 First, IncrementalGenerators have to live in the same ecosystem as all
 the currently extent non-incremental Generator implementations.  Having
 a uniform rebind process whereby the StandardRebindOracle is agnostic to
 this greatly simplified things.  If we were starting from scratch, I
 think it would probably look quite a bit different.

 Standard Generators only return a type-name (which can possibly be
 null).  The RebindResult allows legacy generators to be cleanly wrapped
 and presented to the rebind oracle in a uniform way.

 A RebindResult is not the same as a CachedRebindResult.  The
 StandardRebindOracle uses the info in the RebindResult to decide how to
 proceed.  It may or may not choose to construct a cache entry, and that
 cached entry might contain a partial subset of newly generated types and
 types from a previous cache entry.  Since a generator itself shouldn't
 need to know how to integrate cached types and artifacts, this logic is
 better left up to the StandardRebindOracle.  The RebindResult also
 contains a target type name, and client data, so it's just a container
 to wrap a generators results.

 In the case that an IncrementalGenerator decides it can return quickly
 and request that all previously cached output can be reused, it doesn't
 need to do anything, including not re-committing cached compilation
 units and artifacts to the context, etc.  That's all magic handled by
 the rebind oracle.

 A CachedRebindResult is specifically input to an IncrementalGenerator,
 but it's cumbersome to pass it in as a separate arg to
 generateIncrementally.  Having it live in the GeneratorContext makes one
 less thing a generator implementation needs to pass around to it's
 internal methods (since GeneratorContext at the least is commonly needed
 all over the place for a generator anyway).

 Remember too, that much of the time, an IncrementalGenerator will run
 when generatorResultCaching is not enabled, such as currently for all
 web-mode compiles, so it's more of a contextual thing anyway, which
 makes it more intuitive for it to live in the GeneratorContext.

 I did have a separate GeneratContextExt interface, which extended the
 base GeneratorContext interface, but ultimately this became unnecessary,
 and it greatly simplified the framework by unifying things.

 I think you had a couple other comments offline (but not summarized
 above), I'll wait for your further comments to respond to those.


 http://gwt-code-reviews.**appspot.com/1468804/http://gwt-code-reviews.appspot.com/1468804/




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make generator result caching framework api available publically. (issue1468804)

2011-08-03 Thread Ray Ryan
On Wed, Aug 3, 2011 at 2:36 PM, jbrosenb...@google.com wrote:

 On 2011/08/03 20:03:28, rjrjr wrote:

 Oh, and putClientData seems like a better name.



 Done

  On Wed, Aug 3, 2011 at 1:02 PM, Ray Ryan mailto:rj...@google.com

 wrote:

   I'm still not crazy about having addClientData() and getClientData()

 on

  separate objects. It seems to me that you've violated your own

 principal

  that the GeneratorContext should be the only object that has to get

 passed

  to the generator's helpers.
 
  Can addClientData() move to the context? That would require

 getClientData()

  to be able to retrieve both cached and freshly added data. Is that
  practical? Sounds like it would be hugely convenient.
 


 I'm not sure I see why it makes sense for it not to be part of the
 cached result.  It is indeed closely associated specifically with that
 previously generated result.  By keeping it as part of the cachedResult,
 it makes one less thing that needs to be passed around internally to the
 generator (only the context needs to be passed around).


No, I have two things to pass around. I have to pass around generatorContext
to allow helpers to fetch cached data, and I also have to pass around
something to allow helpers to post new cached data.

That is, in the current scheme I have to write something like:

generateIncrementally(TreeLogger logger, GeneratorContext context, String
typeName) {
  MapString, Serializable helperCachedData;
  new HelperOne(context, helperCachedData).run();
  new HelperTwo(context, helperCachedData).run();

  RebindResult result = new RebindResult(USE_WHATEVER, typeName + Impl);
  for (Map.EntryString, Serializable entry : helperCachedData) {
result.putClientData(entry.key, entry.value);
  }
  return result;
}


instead of

generateIncrementally(TreeLogger logger, GeneratorContext context, String
typeName) {
  new HelperOne(context).run();
  new HelperTwo(context).run();

  return new RebindResult(USE_WHATEVER, typeName + Impl);
}


Further, my helpers have to look in two places for cacheable data generated
by an upstream helper.


 The context is specific to the currently running generator environment,
 and generally gets reset for each generator invocation.  Adding more
 data to the context requires more api and more things for the context to
 keep track of (currently it just keeps track of a CachedRebindResult
 object, which it would still need to do).

 Maybe there's a better name than clientData here, suggestions?  It's
 specific to a generator run invoked that was invoked with the same
 rebind rule and requested type name as are currently in effect.  But
 it's historical data, and in fact is not essential at all to the
 generator (if it's not there, the generator will run to completion).


 http://gwt-code-reviews.**appspot.com/1468804/http://gwt-code-reviews.appspot.com/1468804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make generator result caching framework api available publically. (issue1468804)

2011-08-03 Thread Ray Ryan
LGTM

On Wed, Aug 3, 2011 at 4:17 PM, jbrosenb...@google.com wrote:

 Well, a generator has to keep it's own state anyway, in practice.  In
 the case of AbstractClientBundleGenerator, it actually creates a
 ResourceContext and passes that around, along with the GeneratorContext
 (this was before any IncrementalGenerator enhancements).  In the case of
 RPC, there are 2 SerializableTypeOracleBuilders and the GeneratorContext
 that get passed around.

 I'm not sure, it sounds like you are asking to have a generic
 functionality for the GeneratorContext, so generators can park data
 there as needed (not really specific to IncrementalGenerators).  What
 would the semantics be, would it always get added to the cache in
 entirety, or is there a separate api for saving data for current state
 and another for specifically placing data that needs to be cached?  Is
 it cleared when each new generator invocation occurs?

 Since this change is not attempting to modify how the framework behaves
 with respect to legacy generators, I'd rather not invent api in this
 patch that's not related to the task at hand.

 Using your example, before any IncrementalGenerator work, both
 ClientBundle and RPC look similar to your first example (e.g. there's
 both a GeneratorContext and helperCachedData passed around everywhere).
 The only change I've done is to add a way (without modifying current
 generators), to prepare extra data for subsequent cache reuse checking.
 In fact, the client data stored for cache reuse checking is generally
 newly prepared data, not really needed unless caching is enabled.  So,
 the way things work now, are very similar to the current state, but with
 cache  use support added at the beginning (to check reusability) and at
 the end (to prepare and remember data for future checking).  Like so:


 generateIncrementally(**TreeLogger logger, GeneratorContext context,
 String
 typeName) {

  if (checkCacheReusability(**context))
return new RebindResult(RebindMode.USE_**CACHE,...)

  // this part is no different for non-incremental generators
  MyGeneratorStateClass generatorState;
  new HelperOne(context, generatorState).run();
  new HelperTwo(context, generatorState).run();

  if (canBeCacheable(context)) {
// possibly expensive task
PreparedClientData pcd = prepareClientData(context, helperData);


RebindResult result = new RebindResult(USE_WHATEVER, typeName +
 Impl);
result.putClientData(pcd, pcd);

return result;
  } else {
// no need to prepare any client data
return new RebindResult(RebindMode.USE_**ALL_NEW_WITH_NO_CACHING,...);

  }
 }

 http://gwt-code-reviews.**appspot.com/1468804/http://gwt-code-reviews.appspot.com/1468804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Making lazy widgets generation the default option. (issue1499809)

2011-08-02 Thread Ray Ryan
Lets warn gwt-announce that the default will change next Tuesday, and
encourage them to try it out themselves in the meantime. Are you okay
driving that, and with waiting another week?
On Aug 2, 2011 6:33 AM, her...@google.com wrote:
 Reviewers: rjrjr,

 Description:
 Making lazy widgets generation the default option.

 Review by: rj...@google.com

 Please review this at http://gwt-code-reviews.appspot.com/1499809/

 Affected files:
 M user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
 M user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java


 Index: user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
 ===
 --- user/src/com/google/gwt/uibinder/UiBinder.gwt.xml (revision 10484)
 +++ user/src/com/google/gwt/uibinder/UiBinder.gwt.xml (working copy)
 @@ -31,7 +31,7 @@
 rendering mode and make some widgets lazily created. This is still
 experimental
 but should be the default option in a soon future. --
 define-configuration-property name=UiBinder.useLazyWidgetBuilders
 is-multi-valued=false/
 - set-configuration-property name=UiBinder.useLazyWidgetBuilders
 value=false/
 + set-configuration-property name=UiBinder.useLazyWidgetBuilders
 value=true/

 generate-with class=com.google.gwt.uibinder.rebind.UiBinderGenerator
 when-type-assignable
 class=com.google.gwt.uibinder.client.UiRenderer/
 Index: user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
 ===
 --- user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
 (revision 10484)
 +++ user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
(working
 copy)
 @@ -50,7 +50,7 @@

 private static final String XSS_SAFE_CONFIG_PROPERTY
 = UiBinder.useSafeHtmlTemplates;
 private static final String LAZY_WIDGET_BUILDERS_PROPERTY
 = UiBinder.useLazyWidgetBuilders;
 -
 +
 private static boolean gaveSafeHtmlWarning;

 /**
 @@ -212,7 +212,7 @@
 }

 private Boolean useLazyWidgetBuilders(MortalLogger logger,
 PropertyOracle propertyOracle) {
 - return extractConfigProperty(logger, propertyOracle,
 LAZY_WIDGET_BUILDERS_PROPERTY, false);
 + return extractConfigProperty(logger, propertyOracle,
 LAZY_WIDGET_BUILDERS_PROPERTY, true);
 }

 private Boolean useSafeHtmlTemplates(MortalLogger logger, PropertyOracle
 propertyOracle) {



-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Adding a spot for database column name in Column. Allows us to create an updated SQL statement w... (issue1503806)

2011-08-02 Thread Ray Ryan
I was biting my tongue on this one, but I guess I'll jump in and agree, this
smells bad.

@jlabanca, is there no hook in Column or maybe Cell.Context where this kind
of app-specific data can be added?

On Tue, Aug 2, 2011 at 12:32 PM, Jeff Larsen larse...@gmail.com wrote:

 I'm inclined to agree with Stephen here. No where else in GWT widgetry is
 there a reference to database related things. I don't think this is a big
 deal either, but it seems like a more application specific thing rather than
 something that belongs inside GWT proper.

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Adding a spot for database column name in Column. Allows us to create an updated SQL statement w... (issue1503806)

2011-08-02 Thread Ray Ryan
It's John L's call, but that's certainly my preference.

On Tue, Aug 2, 2011 at 1:10 PM, John Porter Simons porte...@google.comwrote:

 The other way we (me and dramos@) discussed doing this was, in our
 CellTable subclass, have a map from Column to String to store these database
 column names. I can update to that and revert this if you like.


 On Tue, Aug 2, 2011 at 1:01 PM, Ray Ryan rj...@google.com wrote:

 I was biting my tongue on this one, but I guess I'll jump in and agree,
 this smells bad.

 @jlabanca, is there no hook in Column or maybe Cell.Context where this
 kind of app-specific data can be added?

 On Tue, Aug 2, 2011 at 12:32 PM, Jeff Larsen larse...@gmail.com wrote:

  I'm inclined to agree with Stephen here. No where else in GWT widgetry
 is there a reference to database related things. I don't think this is a big
 deal either, but it seems like a more application specific thing rather than
 something that belongs inside GWT proper.

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors





-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add font-face support to CssResource. (issue1502806)

2011-07-26 Thread Ray Ryan
Unnur, would you be comfortable taking this?

On Tue, Jul 26, 2011 at 12:12 PM, b...@google.com wrote:

 Reviewers: rjrjr,

 Message:
 This is a re-spin of a patch that was written back in October but that
 got dropped.  The original patch is at
 http://gwt-code-reviews.**appspot.com/943802http://gwt-code-reviews.appspot.com/943802
 .

 Description:
 Add font-face support to CssResource.
 Patch by: bobv
 Review by: rjrjr


 Please review this at 
 http://gwt-code-reviews.**appspot.com/1502806/http://gwt-code-reviews.appspot.com/1502806/

 Affected files:
  M user/src/com/google/gwt/**resources/css/**CssGenerationVisitor.java
  M user/src/com/google/gwt/**resources/css/GenerateCssAst.**java
  A user/src/com/google/gwt/**resources/css/ast/CssFontFace.**java
  M user/src/com/google/gwt/**resources/css/ast/**CssNodeCloner.java
  M user/src/com/google/gwt/**resources/css/ast/CssVisitor.**java
  M user/test/com/google/gwt/**resources/client/**CSSResourceTest.java
  M user/test/com/google/gwt/**resources/client/test.css




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Implements UiBinder rendering for Cells. (issue1466809)

2011-07-25 Thread Ray Ryan
I've shared a publicly visible copy here:

https://docs.google.com/document/d/1Oo_imxskoGX5O9l9LhHDtJ0yJkHvNTNQqU3dkkekZYI/edit?hl=en_US

Does that work for you?

On Fri, Jul 8, 2011 at 3:22 PM, stephen.haber...@gmail.com wrote:



   Is it okay to make that public?
 I think it is OK. We usually publish them in the Wiki, but Docs is

 getting so good that I wanted to try it this way.

 Using docs makes sense. Will it eventually be made public? Perhaps
 copy/pasted over into the wiki for posterity?




 http://gwt-code-reviews.**appspot.com/1466809/http://gwt-code-reviews.appspot.com/1466809/

 --
 http://groups.google.com/**group/Google-Web-Toolkit-**Contributorshttp://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Remove RenderablaComposite. \o/ (issue1455804)

2011-06-14 Thread Ray Ryan
LGTM

On Tue, Jun 14, 2011 at 10:09 AM, rchan...@google.com wrote:

 On 2011/06/14 16:54:43, rdcastro wrote:

 LGTM


 http://gwt-code-reviews.**appspot.com/1455804/http://gwt-code-reviews.appspot.com/1455804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] RE:Reduce the use of old event bus classes. The only public api that (issue1446819)

2011-06-14 Thread Ray Ryan
Ready for review.


On Tue, Jun 14, 2011 at 5:03 PM, rj...@google.com {subItem.from.uri}
wrote:

 http://gwt-code-reviews.appspot.com/1446819/http://www.google.com/url?sa=Dq=http://gwt-code-reviews.appspot.com/1446819/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: UiBinder - register custom ElementParser (issue1454804)

2011-06-10 Thread Ray Ryan
As soon as we have done that, we can't make changes to UiBinderWriter and
all the other classes the parsers actually talk to, nor can we make sweeping
changes to the code they generate.

If the problem is retrofitting widgets you don't own, would a non-annotation
alternative to UiChild get the job done? Perhaps a config file, perhaps a
builder of some kind?

On Thu, Jun 9, 2011 at 8:16 PM, jus...@jhickman.com wrote:

 Unfortunately @UiChild doesn't handle the flexibility I need, especially
 in cases where you are wanting to write parsers for a 3rd party widget
 library and do not have the ability to modify the source to include
 annotations.

 Is there any way of convincing you to make it more extendable?  Even
 without providing an official mechanism for registering
 ElementParsers, doing small things such as the following would do
 wonders:

 * In UiBinderGenerator, extract the instantiation of the UiBinderWriter
 into a protected method so that  developers can subclass the
 UiBinderGenerator and construct their own subclass of UiBinderWriter

 * In UiBinderWriter, make addElementParser() protected rather than
 private.




 http://gwt-code-reviews.**appspot.com/1454804/http://gwt-code-reviews.appspot.com/1454804/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: UiBinder - register custom ElementParser (issue1454804)

2011-06-10 Thread Ray Ryan
Exactly. And I was thinking we could introduce something like BuildsWidgetW
extends IsWidget extends IsWidget. UiBinder could learn to honor the
setters and such of the underlying widget as well as the BuildsWidget.

On Fri, Jun 10, 2011 at 10:41 AM, Thomas Broyer t.bro...@gmail.com wrote:

 On Fri, Jun 10, 2011 at 7:22 PM, Ray Ryan rj...@google.com wrote:
  As soon as we have done that, we can't make changes to UiBinderWriter and
  all the other classes the parsers actually talk to, nor can we make
 sweeping
  changes to the code they generate.
  If the problem is retrofitting widgets you don't own, would a
 non-annotation
  alternative to UiChild get the job done? Perhaps a config file, perhaps a
  builder of some kind?

 Particularly as you should be able to make an IsWidget class with
 @UiChild methods as a lightweight wrapper around the widget, kind of a
 builder (similar to PotentialElement but at a Widget level).


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Resolve PotentialElement children before inserting them into a container. (issue1454810)

2011-06-10 Thread Ray Ryan
LGTM

On Fri, Jun 10, 2011 at 10:29 AM, jul...@google.com wrote:

 Uploaded patch with assertion in DOM.insertListItem as patch set 3.



 http://gwt-code-reviews.**appspot.com/1454810/diff/3001/**
 user/src/com/google/gwt/user/**client/DOM.javahttp://gwt-code-reviews.appspot.com/1454810/diff/3001/user/src/com/google/gwt/user/client/DOM.java
 File user/src/com/google/gwt/user/**client/DOM.java (right):

 http://gwt-code-reviews.**appspot.com/1454810/diff/3001/**
 user/src/com/google/gwt/user/**client/DOM.java#newcode974http://gwt-code-reviews.appspot.com/1454810/diff/3001/user/src/com/google/gwt/user/client/DOM.java#newcode974
 user/src/com/google/gwt/user/**client/DOM.java:974: String value, int
 index) {
 On 2011/06/10 17:12:23, rjrjr wrote:

 Seems like the assert line should be added here too.


   assert !PotentialElement.isPotential(**selectElem) : Cannot insert

 into a

 PotentialElement;


 Done.


 http://gwt-code-reviews.**appspot.com/1454810/http://gwt-code-reviews.appspot.com/1454810/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make UriUtils#unsafeCastFromUntrustedString gracefully handle null for (issue1443814)

2011-06-07 Thread Ray Ryan
In general we try to be null-intolerant, although I don't know how
consistent we are about it. Basically, nulls should never be quietly cleaned
up for you but rather should fail fast if practical. If null is a legal
value, it should serve a specific purpose.

On Tue, Jun 7, 2011 at 12:44 AM, Thomas Broyer t.bro...@gmail.com wrote:

 On Tue, Jun 7, 2011 at 1:32 AM, Christoph Kern x...@google.com wrote:
  It turns out it was easier to fix the specific case this broke in client
  code (a test that ended up passing null for a URL).
  Which raises the question, should Image gracefully handle null for
 URLs,
  or should the API docs clarify that non-null values are expected?  Is
 there
  a convention for handling nulls in the GWT API?

 SafeHtmlUtils at least doesn't handle 'null' (and will throw NPEs).
 I'd rather have SafeUri follow the same pattern, whether it is to
 throw NPEs or is changed as proposed here for
 unsafeCastFromUntrustedString.


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Does Composite still make sense?

2011-06-07 Thread Ray Ryan
That is an excellent question.

I don't think anyone has yet tried to go that route in earnest, and I
suspect the first to do so will find that IsWidget is not yet as first class
as it should be, just due to undiscovered corner cases and such. But it sure
would be interesting to try to make it work.

On Tue, Jun 7, 2011 at 2:31 AM, Andrés Testi andres.a.te...@gmail.comwrote:

 Since IsWidget is a first class interface, what is the use case to use
 Composite instead of simply implement IsWidget?
 Thanks in advance.

 - Andrés

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Introduce PotentialElement to help with the implementation of IsRenderable widgets. (issue1450810)

2011-06-02 Thread Ray Ryan
On Thu, Jun 2, 2011 at 8:16 AM, jlaba...@google.com wrote:

 LGTM



 http://gwt-code-reviews.appspot.com/1450810/diff/6002/user/src/com/google/gwt/user/client/ui/PotentialElement.java
 File user/src/com/google/gwt/user/client/ui/PotentialElement.java
 (right):


 http://gwt-code-reviews.appspot.com/1450810/diff/6002/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode47
 user/src/com/google/gwt/user/client/ui/PotentialElement.java:47: return
 o...@com.google.gwt.user.client.ui.UIObject::finishBuild()();
 Does referencing o here create a circular reference between
 PotentialElement and UiObject that leads to a memory leak in IE?  I
 think the answer is no, because PotentialElement isn't really a native
 Element, and the IE memory leak is specific to circular references
 between native Elements and logical objects.  Can anyone verify?


I had the same reaction. Everything I found on the issue was specific to dom
objects.



 http://gwt-code-reviews.appspot.com/1450810/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)

2011-05-31 Thread Ray Ryan
LGTM

On Tue, May 31, 2011 at 1:39 PM, jbrosenb...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java
 File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right):


 http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode34
 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:34: * likely
 to succeed.
 On 2011/05/31 16:27:01, rjrjr wrote:

 Why is the argument a string rather than, say, JClassType?



 Since this method will be called from within a generator, there may not
 be a valid JClassType from type oracle (but we can check for the
 availability of a rebind rule, by sourceTypeName, which is how they are
 expressed in rebind rules).  The actual type might come into existence
 after the current (and other) generators complete.


  Wouldn't the second sentence would be more accurate as: is likely to

 succeed

 for a type with no default constructor. (Any concrete type with a zero

 args

 constructor can be instantiated via GWT.create().)



 Can't really check whether a concrete type with a zero args constructor
 might be available, since it might only come into being after other
 generators run.


  Also, it isn't GWT.create(sourceTypeName), the argument is a class

 literal.

 Good point.  I've removed references to GWT.create() altogether in the
 javadoc.


 http://gwt-code-reviews.appspot.com/1450806/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add RequestContext.find() to support chained requests. (issue1448806)

2011-05-27 Thread Ray Ryan
LGTM

On Fri, May 27, 2011 at 7:38 AM, b...@google.com wrote:

  The base interface for RequestFactory service endpoints.
 Add disclaimer explaining that this interface (and the others) are

 normally

 implemented by generated code, and are subject to incompatible

 updates?

 Done.  Also added a FakeRequestContext base type in a testing
 subpackage.


  And should log an item on the issue tracker with the appropriate

 breaking change

 label.


 http://code.google.com/p/google-web-toolkit/issues/detail?id=6413


  Ha! Do you have other sneaky literals like this lying around?


 Calls to the RequestData constructor are usually only in generated code.



 http://gwt-code-reviews.appspot.com/1448806/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Adding new DataGrid widget. DataGrid is a variation of CellTable that supports a fixed header a... (issue1450805)

2011-05-25 Thread Ray Ryan
On Wed, May 25, 2011 at 9:13 AM, Thomas Broyer t.bro...@gmail.com wrote:



 On Wednesday, May 25, 2011 5:46:34 PM UTC+2, Jeff Larsen wrote:



 On Wednesday, May 25, 2011 10:21:18 AM UTC-5, Thomas Broyer wrote:



 On Wednesday, May 25, 2011 3:29:35 PM UTC+2, Jeff Larsen wrote:

 Wow, this is awesome.


 +1


 I haven't started digging into the code yet, but I would like to point
 out a minor nit. In Firefox giving the scrollbars opacity looks OK, but in
 chrome, it doesn't look right (see attached file). Personally, I think
 people are used to not being able to see through scrollbars so I would
 recommend just removing the opacity.


 This was part of another patch a few weeks ago, it's the
 CustomScrollPanel.

 What you're seeing only applies on Windows (IIRC, I had that on Windows
 XP too on the CustomScrollPanel demo that John put online at the time it
 proposed the widget), as it shows well on Ubuntu. It's a Chrome bug that I
 think is not worth working around in GWT.
 See http://code.google.com/p/chromium/issues/detail?id=24524


 Yea, that makes sense that its a chrome bug. I'll just change the css in
 my application to not use any opacity to get around the issue. I think my
 main concern was that most people aren't used to opacity in their
 scrollbars. When I looked at it initially, it just didn't feel like any
 experiences I've had on the web previously.


 Actually, when I saw the transparency, I immediately checked whether it
 wasn't Chrome's default behavior on Linux ;-) (given that I switched from
 Windows less than a week ago, it woudln't have surprised me much that I
 wouldn't have noticed it, as much things I read don't have an horizontal
 scrollbar, and thus wouldn't need transparency on the vertical scrollbar
 either).


 Is there any bug tracker you don't know by heart? :)


 I didn't actually know that bug until 2 mins before sending the previous
 message: I just search on crbug.com for scroll bar opacity. You don't
 need to know things by heart when you have a good search engine ;-)
 (well, I have a good memory, so things regularly ring a bell, and the
 search engine is then just the mean to find it back)

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


 Somehow this seems relevant: http://xkcd.com/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix escaping issue with SafeHtml in Safari3 (issue1443806)

2011-05-25 Thread Ray Ryan
LGTM

On Wed, May 25, 2011 at 11:53 AM, sbruba...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 Fix escaping issue with SafeHtml in Safari3

 Review by: rj...@google.com

 Please review this at http://gwt-code-reviews.appspot.com/1443806/

 Affected files:
  M user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java


 Index: user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
 ===
 --- user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
 (revision 10224)
 +++ user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
 (working copy)
 @@ -631,8 +631,8 @@
   public void testUiTextWithSafeHtml() {
 assertEquals(widgetUi.htmlWithComputedSafeHtml.getHTML().toLowerCase(),
 bthis text should be bold!/b);
 -assertEquals(widgetUi.htmlWithComputedText.getHTML().toLowerCase(),
 -lt;bgt;this text won't be bold!lt;/bgt;);
 +assertEquals(widgetUi.htmlWithComputedText.getHTML().toLowerCase()
 +.replaceAll(, gt;), lt;bgt;this text won't be
 bold!lt;/bgt;);
 assertEquals(widgetUi.labelWithComputedText.getText().toLowerCase(),
 bthis text won't be bold!/b);
   }




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add the method name to the message of InvocationService to make it easier to (issue1450803)

2011-05-24 Thread Ray Ryan
LGTM

On Tue, May 24, 2011 at 5:04 AM, gaill...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 Add the method name to the message of InvocationService to make it
 easier to
 find out the rootcause of the exception.


 Please review this at http://gwt-code-reviews.appspot.com/1450803/

 Affected files:
  M user/src/com/google/gwt/rpc/client/impl/RpcCallbackAdapter.java
  M user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java
  M user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java


 Index: user/src/com/google/gwt/rpc/client/impl/RpcCallbackAdapter.java
 ===
 --- user/src/com/google/gwt/rpc/client/impl/RpcCallbackAdapter.java
 (revision 10205)
 +++ user/src/com/google/gwt/rpc/client/impl/RpcCallbackAdapter.java
 (working copy)
 @@ -83,7 +83,7 @@
 caught = new StatusCodeException(statusCode, encodedResponse);
   } else if (encodedResponse == null) {
 // This can happen if the XHR is interrupted by the server dying
 -caught = new InvocationException(No response payload);
 +caught = new InvocationException(No response payload from  +
 methodName);
   } else {
 result = (T)
 streamFactory.createStreamReader(encodedResponse).readObject();
   }
 Index: user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java
 ===
 --- user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java
  (revision 10205)
 +++ user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java
  (working copy)
 @@ -372,7 +372,8 @@
   return rb.send();
 } catch (RequestException ex) {
   InvocationException iex = new InvocationException(
 -  Unable to initiate the asynchronous service invocation -- check
 the network connection,
 +  Unable to initiate the asynchronous service invocation ( +
 +  methodName + ) -- check the network connection,
   ex);
   callback.onFailure(iex);
 } finally {
 Index:
 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java
 ===
 ---
 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java
  (revision 10205)
 +++
 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java
  (working copy)
 @@ -209,13 +209,13 @@
 caught = new StatusCodeException(statusCode, encodedResponse);
   } else if (encodedResponse == null) {
 // This can happen if the XHR is interrupted by the server dying
 -caught = new InvocationException(No response payload);
 +caught = new InvocationException(No response payload from  +
 methodName);
   } else if (RemoteServiceProxy.isReturnValue(encodedResponse)) {
 result = (T)
 responseReader.read(streamFactory.createStreamReader(encodedResponse));
   } else if (RemoteServiceProxy.isThrownException(encodedResponse)) {
 caught = (Throwable)
 streamFactory.createStreamReader(encodedResponse).readObject();
   } else {
 -caught = new InvocationException(encodedResponse);
 +caught = new InvocationException(encodedResponse +  from  +
 methodName);
   }
 } catch (com.google.gwt.user.client.rpc.SerializationException e) {
   caught = new IncompatibleRemoteServiceException(




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: RequestFactoryServlet

2011-05-24 Thread Ray Ryan
The blog post would be great. We'll plunder from it freely for our own
samples and docs.

One note: lately we've been using Objectify rather than JDO for appengine
apps. It's pretty pleasant.

On Tue, May 24, 2011 at 7:31 AM, Etienne P. etienne...@gmail.com wrote:

 I've gone through the same effort to get this working using Google
 Guice based on some pointers provided by Thomas Broyer in another
 discussion. It works quite well and could be reused easily across
 applications. I've even done some integration with the JDO API to get
 this working in GAE (not without a good amount of trial and error).
 I'll be posting detailed instructions on my blog and committing some
 code to github that people can reuse easily.  Unless there is a better
 place for it in which case I'd be glad to contribute the code there as
 well.

 Suggestions?

 On May 23, 10:14 pm, Jeff Larsen larse...@gmail.com wrote:
  Oh and I did test this to make sure it would with Spring. If you're
  interested here is a link to my test project.
 
  https://github.com/larsenje/SpringRequestFactoryExample

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Public (steph...@gmail.com): (issue1450802)

2011-05-24 Thread Ray Ryan
r10214

On Tue, May 24, 2011 at 6:04 AM, jlaba...@google.com wrote:

 LGTM


 http://gwt-code-reviews.appspot.com/1450802/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Detaching the table section in CellTable before setting the innerHTML to replace the rows. This ... (issue1443805)

2011-05-24 Thread Ray Ryan
Rather than detaching, could you render the new contents in a hidden div and
then replace the tbody?

IE is a little tricky about doing this kind of thing with table elements,
but HTMLPanel.HTMLPanel(String, String) shows the workaround for that (just
render inside a div and all is forgiven).

On Tue, May 24, 2011 at 12:50 PM, jlaba...@google.com wrote:

 In Firefox 3.6 and older, detaching a tbody from a table causes Firefox
 to convert all of the TD elements to divs.  Firefox 4.0 doesn't have
 this problem and works as expected.

 I updated the patch to skip the detach in Firefox 3.6-.


 http://gwt-code-reviews.appspot.com/1443805/

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Detaching the table section in CellTable before setting the innerHTML to replace the rows. This ... (issue1443805)

2011-05-24 Thread Ray Ryan
You're working too hard in IE. You don't need to create the entire table,
you can create fragments so long as you're doing it inside a div

On Tue, May 24, 2011 at 1:21 PM, jlaba...@google.com wrote:

 committed as r10218 before I read Ray's comment.


 On 2011/05/24 20:06:24, rjrjr wrote:

 Rather than detaching, could you render the new contents in a hidden

 div and

 then replace the tbody?

 I suppose that would work too, but the bug is only in Firefox 3.6 and
 earlier.  I'm not sure it would make much of a difference because we
 still end up with a detach, and attach, and an innerHTML call.


I don't follow your logic here. Couldn't you say the same about the entire
optimization? And FF 3.6 is (unfortunately) still a pretty big browser
base.



  IE is a little tricky about doing this kind of thing with table

 elements,

 but HTMLPanel.HTMLPanel(String, String) shows the workaround for that

 (just

 render inside a div and all is forgiven).

 That's an understatement.  You cannot remove a tbody in IE, nor can you
 set its innerHTML.  Instead, we create a new table in a detached div, as
 you suggest, then move every row from the new tbody into the existing
 tbody.


Wow, that's horrible.



  On Tue, May 24, 2011 at 12:50 PM, mailto:jlaba...@google.com wrote:


   In Firefox 3.6 and older, detaching a tbody from a table causes

 Firefox

  to convert all of the TD elements to divs.  Firefox 4.0 doesn't have
  this problem and works as expected.
 
  I updated the patch to skip the detach in Firefox 3.6-.
 
 
  http://gwt-code-reviews.appspot.com/1443805/
 
  --
  http://groups.google.com/group/Google-Web-Toolkit-Contributors
 




 http://gwt-code-reviews.appspot.com/1443805/

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] EventBus package(s) in future GWT releases

2011-05-23 Thread Ray Ryan
We're trying to make bindery the correct location, but we're a bit in
mid-step.

But you shouldn't have to have two instances. gwt...EventBus extends
binder...EventBus.

On Sun, May 22, 2011 at 10:29 AM, dd cafeb...@googlemail.com wrote:

 Hey g-men,

 with GWT 2.3 the autobean, event and requestfactory packages moved from 
 com.google.gwt
 to com.google.web.bindery.
 The 'new' RequestFactory.initialize(EventBus) depends on
 com.google.web.bindery.event.shared.EventBus.
 In contrast the PlaceController.initialize(EventBus) depends on
 com.google.gwt.event.shared.EventBus.
 Using both EventBus packages leads to two @Singleton EventBus instances in
 my application.

 Is EventBus intended to exist in two Packages in future GWT releases or
 will there be one EventBus location?

 Thanks in advance (and for your *great* work)

 Daniel

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Better home for ImageResourceRenderer (issue1446807)

2011-05-20 Thread Ray Ryan
Meh, that's a lot of work for not a lot of gain.

Why don't we just move it to user.client.ui, right next to
AbstractImagePrototype? I don't see why we'd make it a nested class.

On Thu, May 19, 2011 at 10:32 PM, Ray Ryan rj...@google.com wrote:

 I was wondering if we should move the code from abstract image prototype
 here, make it depend on the renderer. Deferred binding and all. What do you
 think?
 On May 19, 2011 9:41 PM, jlaba...@google.com wrote:
 
 
 http://gwt-code-reviews.appspot.com/1446807/diff/1/user/src/com/google/gwt/resources/client/ImageResourceRenderer.java
  File user/src/com/google/gwt/resources/client/ImageResourceRenderer.java
  (right):
 
 
 http://gwt-code-reviews.appspot.com/1446807/diff/1/user/src/com/google/gwt/resources/client/ImageResourceRenderer.java#newcode21
  user/src/com/google/gwt/resources/client/ImageResourceRenderer.java:21:
  import com.google.gwt.user.client.ui.AbstractImagePrototype;
  This creates a dependency from gwt.resources to gwt.user.
 
  Maybe we can make this an inner class of AbsractImagePrototype, since
  that is what is uses for the implementation?
 
  http://gwt-code-reviews.appspot.com/1446807/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: 1. Assert, at runtime, that GWT is running in Standards Mode (i.e. with an appropriate DOCTYPE d... (issue1422816)

2011-05-20 Thread Ray Ryan
LGTM

On Fri, May 20, 2011 at 12:09 PM, jlaba...@google.com wrote:

 LGTM



 http://gwt-code-reviews.appspot.com/1422816/diff/8001/user/src/com/google/gwt/user/client/DocumentModeAsserter.java

 File user/src/com/google/gwt/user/client/DocumentModeAsserter.java
 (right):


 http://gwt-code-reviews.appspot.com/1422816/diff/8001/user/src/com/google/gwt/user/client/DocumentModeAsserter.java#newcode69
 user/src/com/google/gwt/user/client/DocumentModeAsserter.java:69: +
 currentMode + ' is not one of:  + Arrays.toString(allowedModes);
 We should give the user better instructions here.

 Change the doctype at the top of you applications host html page to one
 of  + Arrays.toString(allowedModes) + .  Or, add the following line to
 your gwt.xml file to continue using quirks mode, but understand that GWT
 no longer supports Quirks mode: extend-property
 name=\document.compatMode\ values=\BackCompat\/

 http://gwt-code-reviews.appspot.com/1422816/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Facilitate implementations of EventBus in other packages (issue1443804)

2011-05-20 Thread Ray Ryan
Looks good to me.

Do you still need the patch that brings back GwtEvent.setSource, or is this
enough by itself?

On Thu, May 19, 2011 at 3:58 PM, stephen.haber...@gmail.com wrote:

 Reviewers: rjrjr,

 Description:
 Adds two protected static methods to EventBus that expose otherwise
 inaccessible methods on Event (setSource and dispatch) to subclasses of
 EventBus.

 This is not a huge deal to me, but, in general, it would be nice to move
 my EventBus implementation out of a c.g package. These two methods are
 the key to doing so, and the protected static method approach seemed
 like a reasonable way to expose them to only a small subset of user code
 (namely subclasses of EventBus).


 Please review this at http://gwt-code-reviews.appspot.com/1443804/

 Affected files:
  user/src/com/google/web/bindery/event/shared/EventBus.java


 Index: user/src/com/google/web/bindery/event/shared/EventBus.java
 ===
 --- user/src/com/google/web/bindery/event/shared/EventBus.java  (revision
 10188)
 +++ user/src/com/google/web/bindery/event/shared/EventBus.java  (working
 copy)
 @@ -30,6 +30,24 @@
  */
  public abstract class EventBus {

 +  /** Invokes {@code event.dispatch} with {@code handler}.
 +   *
 +   * This allows EventBus implementations in different packages to
 dispatch
 +   * events even though the {@code event.dispatch} method is protected.
 +   */
 +  protected static H void dispatchEvent(EventH event, H handler) {
 +event.dispatch(handler);
 +  }
 +
 +  /** Sets {@code source} as the source of {@code event}.
 +   *
 +   * This allows EventBus implementations in different packages to set an
 +   * event source even though the {@code event.setSource} method is
 protected.
 +   */
 +  protected static void setSourceOnEvent(Event? event, Object source) {
 +event.setSource(source);
 +  }
 +
   /**
* Adds an unfiltered handler to receive events of this type from all
 sources.
* p


 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Better home for ImageResourceRenderer (issue1446807)

2011-05-19 Thread Ray Ryan
I was wondering if we should move the code from abstract image prototype
here, make it depend on the renderer. Deferred binding and all. What do you
think?
On May 19, 2011 9:41 PM, jlaba...@google.com wrote:


http://gwt-code-reviews.appspot.com/1446807/diff/1/user/src/com/google/gwt/resources/client/ImageResourceRenderer.java
 File user/src/com/google/gwt/resources/client/ImageResourceRenderer.java
 (right):


http://gwt-code-reviews.appspot.com/1446807/diff/1/user/src/com/google/gwt/resources/client/ImageResourceRenderer.java#newcode21
 user/src/com/google/gwt/resources/client/ImageResourceRenderer.java:21:
 import com.google.gwt.user.client.ui.AbstractImagePrototype;
 This creates a dependency from gwt.resources to gwt.user.

 Maybe we can make this an inner class of AbsractImagePrototype, since
 that is what is uses for the implementation?

 http://gwt-code-reviews.appspot.com/1446807/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] GwtEvent.setSource visibility

2011-05-17 Thread Ray Ryan
Yes please, we shouldn't have broken existing code like that.

On Tue, May 17, 2011 at 3:25 PM, Stephen Haberman 
stephen.haber...@gmail.com wrote:

 Hi,

 With the bindery package change, I have a custom EventBus implementation
 that no longer compiles. I was calling GwtEvent.setSource (and squatting
 in the c.g.g.e.shared package to do so), but now setSource is defined in
 the bindery package and so inaccessible.

 I can move my EventBus to the new bindery package and squat there, but
 it makes it complicated supporting both pre-2.4 and post-2.4 codebases.

 If possible, adding a GwtEvent.setSource that restored the old
 visibility would be preferable. I have a simple patch to do so if that's
 okay.

 At some point it would be nice to be able to implement an EventBus
 without squatting in a c.g package, though I understand the appeal of
 keeping the not for user code methods protected.

 - Stephen

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: restore GwtEvent.setSource visibility (issue1450801)

2011-05-17 Thread Ray Ryan
Sneaky. You asked for package protected. I don't want to *expand* the public
foot print.

On Tue, May 17, 2011 at 3:34 PM, stephen.haber...@gmail.com wrote:

 Reviewers: rjrjr,

 Description:
 Redefines Event.setSource in the GwtEvent subclass to restore
 c.g.g.e.shared package visibility.

 Please review this at http://gwt-code-reviews.appspot.com/1450801/

 Affected files:
  user/src/com/google/gwt/event/shared/GwtEvent.java


 Index: user/src/com/google/gwt/event/shared/GwtEvent.java
 ===
 --- user/src/com/google/gwt/event/shared/GwtEvent.java  (revision 10188)
 +++ user/src/com/google/gwt/event/shared/GwtEvent.java  (working copy)
 @@ -101,6 +101,10 @@
 setSource(null);
   }

 +  protected void setSource(Object source) {
 +super.setSource(source);
 +  }
 +
   void overrideSource(Object source) {
 super.setSource(source);
   }




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] rfc on 6068 fix

2011-05-09 Thread Ray Ryan
Bob V is the authority here, but he's on vacation this week.

On Sun, May 8, 2011 at 7:08 AM, Patrick Julien pjul...@gmail.com wrote:

 Here is what I am planning to do to fix issue 6068

 http://code.google.com/p/google-web-toolkit/issues/detail?id=6068

 1. Add to BeanMethod.java (package
 com.google.web.bindery.autobean.vm.impl;) an TO_ARRAY value

 2. Add to JBeanMethod.java
 (com.google.web.bindery.autobean.gwt.rebind.model;) an TO_ARRAY value

 3. in AutoBeanFactoryGenerator#writeShim (package
 com.google.web.bindery.autobean.gwt.rebind;) handle the TO_ARRAY case
 by checking the:

 a. if the type is assignable to Collection?
 b. Determine if toArray has one of two supported method signatures in
 Collection?
 c. Generate code that assigns each element of the newly created array
 by wrapping it first

 4. Add a unit test to check for unfrozen beans.

 a. Get a collection of AutoBeans
 b. assert they are frozen
 c. Initialize an ArrayList using this other collection
 d. Assert the objects stored the second ArrayList are unfrozen


 Open Questions:

 1. Should I try to handle immutable collections, i.e.,
 Collections.unmodifiable* or guava's Immutable*?

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Better error reporting (issue1444801)

2011-05-09 Thread Ray Ryan
Verbal LGTM from John.

r10167

On Mon, May 9, 2011 at 1:40 PM, rj...@google.com wrote:

 http://gwt-code-reviews.appspot.com/1444801/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] AssertionError break Scheduler

2011-05-06 Thread Ray Ryan
Filing a bug would be helpful, thanks.

If you want to offer a fix the details are at
http://code.google.com/webtoolkit/makinggwtbetter.html
On May 6, 2011 12:50 AM, Laurent Le Goff legoff.laur...@gmail.com wrote:

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Eclipse configuration for MobileWebApp. (issue1437801)

2011-05-06 Thread Ray Ryan
On Fri, May 6, 2011 at 7:52 AM, rchan...@google.com wrote:

 On 2011/05/05 22:15:56, rjrjr wrote:

 Rietveld is choking on this.


 Yes, something is broken in my git5 client that plays badly with
 reitveld.


  The non-eclipse instructions in the README.txt are wrong, aren't they?

 ant war can't work because

 of the missing appengine jars. Shouldn't we be saying that the sample

 is configured to assume you're using GPE?

 Has been working reliably for me. I actually use it to detect when
 things fail because Eclipse setup has broken.

 Can you try setting up a local.properties file pointing to App Engine
 SDK?



 # Starting from trunk
 cd samples/mobilewebapp
 vim local.properties #Define gwt.sdk and appengine.sdk



 ant -f user-build.xml war
 ant -f user-build.xml devmode

 This will work but for server-side validation because war/WEB-INF/lib
 does not contain hibernate-validation, slf4j-api, slf4j-log4j and log4j.
 But the distributed app does get the necessary jars in war/WEB-INF/lib
 and produce a proper war.


Cool. How?



 http://gwt-code-reviews.appspot.com/1437801/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Eclipse configuration for MobileWebApp. (issue1437801)

2011-05-06 Thread Ray Ryan
LGTM, by the way.

On Fri, May 6, 2011 at 9:30 AM, Rodrigo Chandia rchan...@google.com wrote:

 On Fri, May 6, 2011 at 12:09 PM, Ray Ryan rj...@google.com wrote:

 But the distributed app does get the necessary jars in war/WEB-INF/lib
 and produce a proper war.


 Cool. How?


 'samples/build.xml' calls 'samples/mobilewebapp/ant.xml' with target
 'source+libs'. That target copies all necessary files and libs from tools.
 Several files get renamed:

 user-build.xml - build.xml
 user-classpath - .classpath
 user-project - .project
 user-settings/ - .settings/

 For example:

 # Starting from trunk
 ant dist-dev # to get an updated GWT in build/staging
 cd samples/mobilewebapp
 ant # copies and renames files
 cd build/out/samples/MobileWebApp
 ls war/WEB-INF/lib # contains jars from tools

 # Either use ant
 vim local.properties # define gwt.sdk and appengine.sdk
 ant war
 ant devmode # no 'ant deploy' because I was lazy

 # OR use eclipse
 1. Import Existing projects...
 2. Fix the appengine and GWT JAR warnings
 3. Run/Debug as Web Application (clean you launch configs, just in case)
 4. GWT compile
 5. Google - AppEngine deploy

 In the distributed gwt-x.y.z.zip gwt.sdk already points to '../../' but
 local.properties still needs to define 'appengine.sdk'

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Improving SimpleAppCacheLinker noise. (issue1438802)

2011-05-06 Thread Ray Ryan
LGTM

On Fri, May 6, 2011 at 12:13 PM, rchan...@google.com wrote:

 OK, I promise I'll try to fix my rietveld reviews. Please use the
 Unified diff view for the time being.


 On 2011/05/06 19:01:27, rchandia wrote:



 http://gwt-code-reviews.appspot.com/1438802/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Eclipse configuration for MobileWebApp. (issue1437801)

2011-05-05 Thread Ray Ryan
Rietveld is choking on this.

The non-eclipse instructions in the README.txt are wrong, aren't they? ant
war can't work because
of the missing appengine jars. Shouldn't we be saying that the sample
is configured to assume you're using GPE?

On Thu, May 5, 2011 at 2:59 PM, rchan...@google.com wrote:

 Reviewers: cramsdale, rjrjr,

 Description:
 Eclipse configuration for MobileWebApp.
 Relies on AppEngine and GPE
 README.txt instructions updated to help setting up Eclipse in this case


 Please review this at http://gwt-code-reviews.appspot.com/1437801/

 Affected files:
  A eclipse/samples/MobileWebApp/.checkstyle
  A eclipse/samples/MobileWebApp/.classpath
  A eclipse/samples/MobileWebApp/.project
  A
 eclipse/samples/MobileWebApp/.settings/com.google.appengine.eclipse.core.prefs
  A eclipse/samples/MobileWebApp/.settings/com.google.gdt.eclipse.core.prefs
  M samples/build.xml
  M samples/common.ant.xml
  M samples/expenses/build.xml
  M samples/mobilewebapp/README.txt
  M samples/mobilewebapp/build.xml
  M
 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/desktop/DesktopTaskListView.java
  M
 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/mobile/MobileTaskListView.java
  M samples/mobilewebapp/user-build.xml
  A samples/mobilewebapp/user-classpath
  A samples/mobilewebapp/user-project
  A
 samples/mobilewebapp/user-settings/com.google.appengine.eclipse.core.prefs
  M samples/mobilewebapp/war/WEB-INF/appengine-web.xml
  A samples/mobilewebapp/war/WEB-INF/classes/marker




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)

2011-05-05 Thread Ray Ryan
Rodrigo, it looks like John is on vacation for the rest of the week. Can you
finish this review?

On Thu, May 5, 2011 at 2:24 PM, rj...@google.com wrote:

 Ready for another look.

 OrientationMonitor is replaced with OrientationHelper. John, I think
 I've stumbled onto a pretty nice widget plugin pattern there. I was able
 to delete the superclass for the shells without adding much boilerplate
 or copy/paste. What do you think?

 Notice that I've snuck a method onto the HasAttachHandlers interface. I
 think we can get away with that since it's meant to be implemented by
 Widget — I'm willing to break a few tests to make it more useful.

 Hmm. I suppose I could instead introduce an interface into the demo
 code, IsAttachable extends HasAttachHandlers. The change sure feels
 right, though.




 http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
 File

 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
 (right):


 http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java#newcode50

 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java:50:
 * @return the local {@link Storage} object, or null unsupporting
 browsers
 On 2011/05/04 20:29:44, jlabanca wrote:

 /r/null unsupporting/null in unsupporting


 Done.


 http://gwt-code-reviews.appspot.com/1433801/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Force locale to en_US for user unit tests (issue1430801)

2011-05-04 Thread Ray Ryan
This is probably html unit honoring the locale and changing its behavior
from what the tests expect.
On May 4, 2011 2:02 AM, t.bro...@gmail.com wrote:
 On 2011/05/03 17:51:18, rjrjr wrote:
 Running ant clean dist-dev test, this appears to break the i18n suite
 under
 html unit.

 Oops! Only tested with a -Dgwt.junit.testcase.includes (which obviously
 didn't include I18N tests)

 That seems weird though that simply running the JVM in a different
 locale suddenly breaks tests of things that are emulated.

 Testcase: testMessageDateTime took 0.15 sec
 FAILED
 Remote test failed at 172.31.131.172 / Mozilla/5.0 (Windows; U;
 Windows NT
 5.1; en-US; rv:1.9.0.19) Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15
 junit.framework.AssertionFailedError: Remote test failed at
 172.31.131.172 /
 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.19)
 Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15

 Actually, 3 tests always fail for me, whichever locale I'm running them
 in (including using es_MX, or removing the -Duser.language/user.region
 system props altogether!)
 And they always fail with the exact same error; so it looks more like a
 regression in the I18N code (particularly as all other I18N tests pass).

 test.dev.htmlunit:
 [echo] Writing test results to C:\Documents and Settings\tbr\Mes
 documents\Mes Projets\gwt\trunk\build\out\user\test/dev-htmlunit/reports
 for test.dev.htmlunit.tests
 [junit] WARNING: multiple versions of ant detected in path for junit

 [junit]

jar:file:/C:/eclipse/plugins/org.apache.ant_1.7.1.v20100518-1145/lib/ant.jar!/org/apache/tools/ant/Project.class
 [junit] and

jar:file:/C:/Documents%20and%20Settings/tbr/Mes%20documents/Mes%20Projets/gwt/trunk/build/lib/gwt-dev.jar!/org/apache/tools/ant/Project.class
 [junit] Running com.google.gwt.i18n.I18NSuite
 [junit] expected=It is Feb 15, 2010 actual=It is 2010 Feb 15)
 [junit] expected=Es ist 15. Feb 2010 actual=It is 2010 Feb 15)
 [junit] expected=Short: 2010-02-01 actual=Short: 2010-02-02)


 http://gwt-code-reviews.appspot.com/1430801/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: 1. Assert, at runtime, that GWT is running in Standards Mode (i.e. with an appropriate DOCTYPE d... (issue1422816)

2011-05-04 Thread Ray Ryan
Please tell me that this is the last step in the following chain. It seems
unlikely if you're only providing the property now.

First you provide the property to allow quirks, and give compiler warnings
that only go away if you set standards mode or set the quirks property. (And
publicize that it will be the default in the next release.) Then you change
the default and break existing apps.

This change should also include updates to the javadoc of widgets that
require quirks to point to the new property.
 On May 4, 2011 7:38 AM, jlaba...@google.com wrote:
 This is a breaking change for existing apps that run in quirks mode.
 I'm alright with that because I don't see an alternative, but we'll need
 to call if out in the release notes and tell people the workaround.



http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml
 File user/src/com/google/gwt/user/UserAgent.gwt.xml (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode29
 user/src/com/google/gwt/user/UserAgent.gwt.xml:29:
 extra newline


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode56
 user/src/com/google/gwt/user/UserAgent.gwt.xml:56:
 define-configuration-property name=document.compatMode
 I think we should break this out into a separate DocumentMode.gwt.xml
 file and inherit it in User.gwt.xml. It isn't part of the UserAgent.


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode67
 user/src/com/google/gwt/user/UserAgent.gwt.xml:67:
 extra newline


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java
 File user/src/com/google/gwt/user/client/DocumentModeAsserter.java
 (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java#newcode27
 user/src/com/google/gwt/user/client/DocumentModeAsserter.java:27: *
 rendering mode is of of the values allowed by the
 /r/of of/one of


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
 File user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
 (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode49
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:49:
 logger.log(TreeLogger.ERROR, OOPS, e);
 Maybe something better than OOPS


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode57
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:57:
 JClassType remoteService = typeOracle.findType(typeName);
 Isn't removeService the same as userType?


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode76
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:76:
 logger.log(TreeLogger.WARN, Unable to find value for '
 If we are going to throw an exception, this should be an ERROR instead
 of a WARN


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode102
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:102: for
 (IteratorString iterator = documentModes.iterator();
 iterator.hasNext();) {
 You can shorten this to:
 for (String next : documentModes) {


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode103
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:103:
 sw.println(\ + iterator.next() + \, );
 This array will always end with a comma. Does Java handle that
 correctly?
 return new String[]{a,b,};

 http://gwt-code-reviews.appspot.com/1422816/

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread Ray Ryan
I mean that you should have been able to fix it by replace place1 == place2
with place1.equals(place2).

On Wed, May 4, 2011 at 8:39 AM, jlaba...@google.com wrote:

 @rjrjr -

 What do you mean?


 http://gwt-code-reviews.appspot.com/1428810/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread Ray Ryan
Also, this change is a stark reminder that we need to get real about request
caching in RequestFactory. You fundamentally shouldn't be doing this, you
should be able to make the redundant request counting on a lower layer to
send it only if necessary.

Bob, I've lost track -- are there hooks in place yet that we could implement
app specific client side caching in a sample like this?

On Wed, May 4, 2011 at 8:56 AM, Ray Ryan rj...@google.com wrote:

 I mean that you should have been able to fix it by replace place1 == place2
 with place1.equals(place2).


 On Wed, May 4, 2011 at 8:39 AM, jlaba...@google.com wrote:

 @rjrjr -

 What do you mean?


 http://gwt-code-reviews.appspot.com/1428810/




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread Ray Ryan
On Wed, May 4, 2011 at 9:47 AM, Bob Vawter robertvaw...@google.com wrote:

  Bob, I've lost track -- are there hooks in place yet that we could
 implement
  app specific client side caching in a sample like this?

 You can call RequestFactory.getSerializer() with an implementation of
 a ProxyStore (e.g. DefaultProxyStore) in order to indefinitely persist
 a proxy.  Beyond simple serialization, there's no mechanism in place
 to short-circuit requests.


That's proxies. Can requests be serialized? For that matter, any reason not
to just hold on to them? I'm not talking about LocalStorage or anything
here, just optimizing within a single session.

I know we can extend or replace DefaultRequestTransport on the client side,
e.g. as done in Expenses for GAE
integrationhttps://cs.corp.google.com/#google3/third_party/java_src/gwt/svn/trunk/samples/expenses/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java.
If requests have reasonable equality semantics and are somewhat
introspectable, even just at the instanceof level, we might be able to get
some simple caching done in this sample.

@jlabanca, I know your time is short and you gotta do what you gotta do. I
don't mean to hold up this change for caching. I just want to have the
conversation while we have a use case staring us in the face.



 --
 Bob Vawter
 Google Web Toolkit Team


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

2011-05-04 Thread Ray Ryan
I'd be inclined to start with a) and see what happens.

On Wed, May 4, 2011 at 10:02 AM, rdcas...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode205
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:205: if
 (isFullyInitialized()) {
 On 2011/05/04 16:47:17, rjrjr wrote:

 This seems pretty unexpected. I would have thought it would either be

 an error

 (RuntimeException), or else that I would do the opposite of this:

 replace my

 existing element with the new one.


 I'd be totally fine with a RuntimeError, it'll probably be m ore
 manageable in the long-term. I was trying to support this because even
 though it's less efficient, we can still make it work. The scenario here
 is:

 1-) Build Attachable tree. This includes calling all the render() stuff
 and so on. Let's assume that this particular AttachableHTMLPanel is in
 the middle of the tree.

 2-) For some reason do something to this particular panel (that's in the
 middle of the tree) that calls its getElement() [let's say attach it to
 the document]. This triggers the process of building the widget tree as
 if this panel was the root (i.e., hidden div, set innerHTML, getting
 elements for all children and initializing them). At this point
 everything works

 3-) Now you go and attach the real root of the tree, an ancestor of
 this panel. We can do one of 3 things:
  (a) throw an error. You probably didn't mean to do this as it's less
 efficient
  (b) replace the element that our parent assigned us (along with its
 subtree) with the subtree we already built in step 2
  (c) re-do everything in step 2, ignoring the fact that we already
 initialized all children widget

 I think (a) would be ideal, specially for debugging purposes. Honestly I
 implemented (b) because I was a little afraid and wanted to be as
 backwards-compatible as possible.  I don't think (c) would work
 out-of-the-box, as we'd have to support initializing Widgets twice
 (i.e., re-setting the element, etc.)

 Makes sense?


 http://gwt-code-reviews.appspot.com/1427812/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread Ray Ryan
Something as simple as short circuiting all equivalent
RequestFactory.find(EntityProxyIdP) requests, where equivalent takes into
account the proxy version as well as its id, could be very effective.

On Wed, May 4, 2011 at 10:04 AM, John LaBanca jlaba...@google.com wrote:

 On Wed, May 4, 2011 at 12:58 PM, Ray Ryan rj...@google.com wrote:

 On Wed, May 4, 2011 at 9:47 AM, Bob Vawter robertvaw...@google.comwrote:

  Bob, I've lost track -- are there hooks in place yet that we could
 implement
  app specific client side caching in a sample like this?

 You can call RequestFactory.getSerializer() with an implementation of
 a ProxyStore (e.g. DefaultProxyStore) in order to indefinitely persist
 a proxy.  Beyond simple serialization, there's no mechanism in place
 to short-circuit requests.


 That's proxies. Can requests be serialized? For that matter, any reason
 not to just hold on to them? I'm not talking about LocalStorage or anything
 here, just optimizing within a single session.

 I know we can extend or replace DefaultRequestTransport on the client
 side, e.g. as done in Expenses for GAE 
 integrationhttps://cs.corp.google.com/#google3/third_party/java_src/gwt/svn/trunk/samples/expenses/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java.
 If requests have reasonable equality semantics and are somewhat
 introspectable, even just at the instanceof level, we might be able to get
 some simple caching done in this sample.

 @jlabanca, I know your time is short and you gotta do what you gotta do. I
 don't mean to hold up this change for caching. I just want to have the
 conversation while we have a use case staring us in the face.

 This change was already submitted, but a more comprehensive solution would
 be nice.



 --

 Bob Vawter
 Google Web Toolkit Team





-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

2011-05-04 Thread Ray Ryan
Once we've validated the work, seems like a lot of the Attachable support
should be baked into UiObject, Widget and Panel in some kind of opt-in
manner.

On Wed, May 4, 2011 at 10:20 AM, Rafael Castro rdcas...@google.com wrote:

 Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
 this already works pretty well. I have to review some other tricky cases
 (like if you add a non-attachable widget to an Attachable panel before
 finishing the initialization), but we're pretty close. The other cases that
 could trigger this is calling some UIObject method that we haven't yet
 @Override (like we did for setStyleName). Those call getElement() and hurt
 us.


 On Wed, May 4, 2011 at 2:15 PM, rj...@google.com wrote:

 How's that?

 Is the bit I wrote about after adding it to a panel accurate? Seems
 like we're trying to get to a world where the add would be fine, and the
 wrap call  wouldn't happen until the parent is wrapped — are we there
 already?



 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java

 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode211
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:211:
 throw new IllegalStateException(
 wrapElement() cannot be called twice, or after a call to getElement()
 has forced the widget to render itself (e.g. after adding it to a panel)


 http://gwt-code-reviews.appspot.com/1427812/


  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)

2011-05-04 Thread Ray Ryan
I guess I'm speaking strictly of the null checks. It's fine for our sample
code not to have a real auth system in its storage, of course.

Seems like your UserServiceWrapper should have a requireCurrentUserId()
method that throws an exception if there is no id, and Task shouldn't be
friendly. Is that reasonable?

On Wed, May 4, 2011 at 11:19 AM, rj...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
 File


 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
 (right):


 http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java#newcode48

 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java:48:
 }
 This seems bad. I'd never allow code like this in a production app. An
 unauthorized user shouldn't even be able to reach this class.


 http://gwt-code-reviews.appspot.com/1432801/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Force locale to en_US for user unit tests (issue1430801)

2011-05-03 Thread Ray Ryan
LGTM

Thanks, will submit.

On Tue, May 3, 2011 at 2:40 AM, t.bro...@gmail.com wrote:

 Reviewers: rjrjr, bobv,

 Description:
 Force locale to en_US for user unit tests

 Force locale to en_US for user unit tests, otherwise validation tests
 fail (use hard-coded checks on locale-dependent messages)

 Please review this at http://gwt-code-reviews.appspot.com/1430801/

 Affected files:
  M user/build.xml


 Index: user/build.xml
 diff --git a/user/build.xml b/user/build.xml
 index
 4f6286fd2e654d9b63ad55bc393807b23954c084..e7696fff725e9e66ee2072d7975173bbc420f178
 100755
 --- a/user/build.xml
 +++ b/user/build.xml
 @@ -2,7 +2,8 @@
   property name=gwt.root location=.. /
   property name=project.tail value=user /
   property name=test.args value=-ea /
 -  property name=test.jvmargs value=-ea /
 +  !-- Bean validation and RequestFactory tests use hard-coded checks on
 locale-dependent messages --
 +  property name=test.jvmargs value=-ea -Duser.language=en
 -Duser.region=US /

   !-- support old variables names --
   condition property=gwt.hosts.web.remote
 value=${gwt.remote.browsers}
 @@ -763,7 +764,7 @@
   excludes=${gwt.tck.testcase.dev.excludes} /
   gwt.junit test.name=test.dev.htmlunit
   test.args=${test.args} -standardsMode 
 -  test.jvmargs=-ea
 -Dcom.google.gwt.sample.validationtck.util.Failing.include=true
  -Dcom.google.gwt.sample.validationtck.util.NonTckTest.exclude=true
 +  test.jvmargs=${test.jvmargs}
 -Dcom.google.gwt.sample.validationtck.util.Failing.include=true
  -Dcom.google.gwt.sample.validationtck.util.NonTckTest.exclude=true
   test.out=${junit.out}/tck-dev-htmlunit
   test.cases=tck.dev.htmlunit.tests
   haltonfailure=false




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Promoting LazyDomElement to be used externally. LazyDomElement can be (issue1427809)

2011-05-03 Thread Ray Ryan
On Tue, May 3, 2011 at 9:49 AM, her...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java
 File user/src/com/google/gwt/uibinder/client/LazyDomElement.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode33
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:33: *
 lt;div ui:field=myDiv class={anyClass}/gt;
 On 2011/05/02 16:51:54, rjrjr wrote:

 The class bit is noise, can you snip it?


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode45
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:45: */

 On 2011/05/02 16:51:54, rjrjr wrote:

 /**
   * Creates an instance to fetch the element with the given id.
   */


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode54
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:54:

 On 2011/05/02 16:51:54, rjrjr wrote:

 /**
   * Returns the dom element
   * @return the dom element
   * @throws RuntimeException is the element cannot be found
   */


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode59
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:59: throw
 new RuntimeException(Element is not attached.);
 On 2011/05/02 16:51:54, rjrjr wrote:

 Cannot find element with id \ + domId + \. Perhaps it is not

 attached to

 the document body?


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java
 File
 user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java#newcode46

 user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java:46:
 if (!templateFieldType.isAssignableTo(parameterType)) {
 On 2011/05/02 16:51:54, rjrjr wrote:

 Needs a unit test. See FieldWriterOfGeneratedCssResourceTest for

 example.

 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
 File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode380
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:380: // But
 if the owner field is an instance of LazyDomElement then the code
 On 2011/05/02 16:51:54, rjrjr wrote:

 /* for long comments


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode463
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:463: // We
 can switch types if useLazyWidgetBuilders is enabled and the
 On 2011/05/02 16:51:54, rjrjr wrote:

 /*


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode710
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:710: return
 lazyDomElementClass.isAssignableFrom(ownerField.getType().getRawType());
 On 2011/05/02 16:51:54, rjrjr wrote:

 Can getRawType can return null? Is isAssignableFrom null safe?


 Nop, OwnerField dies if it can't resolve the type.



 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java
 File user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java#newcode85
 user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java:85: public
 JClassType getRawType() {
 On 2011/05/03 16:37:02, rjrjr wrote:

 Please inline this.


 You meant remove this method? This is very handy for tests.


Oh, I see. Could you put a comment in the method body to that effect?




 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java
 File
 user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java#newcode22

 user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java:22:
 public class FieldWriterOfExistingTypeTest extends TestCase {
 On 2011/05/03 16:37:02, rjrjr wrote:

 I forgot how handy EasyMock is


 True.


 http://gwt-code-reviews.appspot.com/1427809/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Promoting LazyDomElement to be used externally. LazyDomElement can be (issue1427809)

2011-05-03 Thread Ray Ryan
LGTM

On Tue, May 3, 2011 at 9:54 AM, Ray Ryan rj...@google.com wrote:



 On Tue, May 3, 2011 at 9:49 AM, her...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java
 File user/src/com/google/gwt/uibinder/client/LazyDomElement.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode33
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:33: *
 lt;div ui:field=myDiv class={anyClass}/gt;
 On 2011/05/02 16:51:54, rjrjr wrote:

 The class bit is noise, can you snip it?


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode45
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:45: */

 On 2011/05/02 16:51:54, rjrjr wrote:

 /**
   * Creates an instance to fetch the element with the given id.
   */


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode54
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:54:

 On 2011/05/02 16:51:54, rjrjr wrote:

 /**
   * Returns the dom element
   * @return the dom element
   * @throws RuntimeException is the element cannot be found
   */


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/client/LazyDomElement.java#newcode59
 user/src/com/google/gwt/uibinder/client/LazyDomElement.java:59: throw
 new RuntimeException(Element is not attached.);
 On 2011/05/02 16:51:54, rjrjr wrote:

 Cannot find element with id \ + domId + \. Perhaps it is not

 attached to

 the document body?


 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java
 File
 user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java#newcode46

 user/src/com/google/gwt/uibinder/rebind/FieldWriterOfLazyDomElement.java:46:
 if (!templateFieldType.isAssignableTo(parameterType)) {
 On 2011/05/02 16:51:54, rjrjr wrote:

 Needs a unit test. See FieldWriterOfGeneratedCssResourceTest for

 example.

 Done.


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
 File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode380
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:380: // But
 if the owner field is an instance of LazyDomElement then the code
 On 2011/05/02 16:51:54, rjrjr wrote:

 /* for long comments


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode463
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:463: // We
 can switch types if useLazyWidgetBuilders is enabled and the
 On 2011/05/02 16:51:54, rjrjr wrote:

 /*


 Done.



 http://gwt-code-reviews.appspot.com/1427809/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode710
 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:710: return
 lazyDomElementClass.isAssignableFrom(ownerField.getType().getRawType());
 On 2011/05/02 16:51:54, rjrjr wrote:

 Can getRawType can return null? Is isAssignableFrom null safe?


 Nop, OwnerField dies if it can't resolve the type.



 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java
 File user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java#newcode85
 user/src/com/google/gwt/uibinder/rebind/model/OwnerField.java:85: public
 JClassType getRawType() {
 On 2011/05/03 16:37:02, rjrjr wrote:

 Please inline this.


 You meant remove this method? This is very handy for tests.


 Oh, I see. Could you put a comment in the method body to that effect?




 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java
 File

 user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java
 (right):


 http://gwt-code-reviews.appspot.com/1427809/diff/1010/user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java#newcode22

 user/test/com/google/gwt/uibinder/rebind/FieldWriterOfExistingTypeTest.java:22:
 public class FieldWriterOfExistingTypeTest extends TestCase {
 On 2011/05/03 16:37:02, rjrjr wrote:

 I forgot how handy EasyMock is


 True.


 http://gwt-code-reviews.appspot.com/1427809/




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Force locale to en_US for user unit tests (issue1430801)

2011-05-03 Thread Ray Ryan
Running ant clean dist-dev test, this appears to break the i18n suite under
html unit.

Testcase: testMessageDateTime took 0.15 sec
FAILED
Remote test failed at 172.31.131.172 / Mozilla/5.0 (Windows; U; Windows NT
5.1; en-US; rv:1.9.0.19) Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15
junit.framework.AssertionFailedError: Remote test failed at 172.31.131.172 /
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.19)
Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15


On Tue, May 3, 2011 at 9:42 AM, Ray Ryan rj...@google.com wrote:

 LGTM

 Thanks, will submit.


 On Tue, May 3, 2011 at 2:40 AM, t.bro...@gmail.com wrote:

 Reviewers: rjrjr, bobv,

 Description:
 Force locale to en_US for user unit tests

 Force locale to en_US for user unit tests, otherwise validation tests
 fail (use hard-coded checks on locale-dependent messages)

 Please review this at http://gwt-code-reviews.appspot.com/1430801/

 Affected files:
  M user/build.xml


 Index: user/build.xml
 diff --git a/user/build.xml b/user/build.xml
 index
 4f6286fd2e654d9b63ad55bc393807b23954c084..e7696fff725e9e66ee2072d7975173bbc420f178
 100755
 --- a/user/build.xml
 +++ b/user/build.xml
 @@ -2,7 +2,8 @@
   property name=gwt.root location=.. /
   property name=project.tail value=user /
   property name=test.args value=-ea /
 -  property name=test.jvmargs value=-ea /
 +  !-- Bean validation and RequestFactory tests use hard-coded checks on
 locale-dependent messages --
 +  property name=test.jvmargs value=-ea -Duser.language=en
 -Duser.region=US /

   !-- support old variables names --
   condition property=gwt.hosts.web.remote
 value=${gwt.remote.browsers}
 @@ -763,7 +764,7 @@
   excludes=${gwt.tck.testcase.dev.excludes} /
   gwt.junit test.name=test.dev.htmlunit
   test.args=${test.args} -standardsMode 
 -  test.jvmargs=-ea
 -Dcom.google.gwt.sample.validationtck.util.Failing.include=true
  -Dcom.google.gwt.sample.validationtck.util.NonTckTest.exclude=true
 +  test.jvmargs=${test.jvmargs}
 -Dcom.google.gwt.sample.validationtck.util.Failing.include=true
  -Dcom.google.gwt.sample.validationtck.util.NonTckTest.exclude=true
   test.out=${junit.out}/tck-dev-htmlunit
   test.cases=tck.dev.htmlunit.tests
   haltonfailure=false





-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix Attachable for those poor fellows who don't have the bliss of SafeHtml enabled (yet). (issue1426808)

2011-05-02 Thread Ray Ryan
LGTM

On Fri, Apr 29, 2011 at 2:06 PM, rdcas...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 Fix Attachable for those poor fellows who don't have the bliss of
 SafeHtml enabled (yet).


 Please review this at http://gwt-code-reviews.appspot.com/1426808/

 Affected files:
  M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java


 Index: user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
 ===
 --- user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (revision
 10113)
 +++ user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (working
 copy)
 @@ -792,7 +792,7 @@
*/
   public String tokenForSafeHtmlExpression(String expression) {
 if (!useSafeHtmlTemplates) {
 -  return tokenForStringExpression(expression);
 +  return tokenForStringExpression(expression + .asString());
 }

 String token = tokenator.nextToken(expression);




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. (issue1427810)

2011-05-02 Thread Ray Ryan
This still appears to have all the problems of
http://gwt-code-reviews.appspot.com/1426803.

On Mon, May 2, 2011 at 11:33 AM, rchan...@google.com wrote:

 Reviewers: rjrjr,

 Description:
 SafeHtmlRenderer code gen for UiBinder.
 Picking-up patch from rietveld issue 1426803


 Please review this at http://gwt-code-reviews.appspot.com/1427810/

 Affected files:
  M user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
  M user/src/com/google/gwt/uibinder/elementparsers/CustomButtonParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/DialogBoxParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/DomElementParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/GridParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/HTMLPanelParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/HasHTMLParser.java
  M
 user/src/com/google/gwt/uibinder/elementparsers/StackLayoutPanelParser.java
  M
 user/src/com/google/gwt/uibinder/elementparsers/TabLayoutPanelParser.java
  M user/src/com/google/gwt/uibinder/elementparsers/TabPanelParser.java
  M user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
  M user/src/com/google/gwt/uibinder/rebind/FieldManager.java
  M user/src/com/google/gwt/uibinder/rebind/FieldWriter.java
  M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

  1   2   3   4   5   6   7   8   >