[gwt-contrib] Re: This patch makes three changes: (issue1093801)

2010-11-09 Thread Amit Manjhi
On Tue, Nov 9, 2010 at 12:32 PM, j...@google.com wrote:


 http://gwt-code-reviews.appspot.com/1093801/diff/1/2
 File tools/api-checker/config/gwt20_21userApi.conf (right):

 http://gwt-code-reviews.appspot.com/1093801/diff/1/2#newcode99
 tools/api-checker/config/gwt20_21userApi.conf:99:
 :com.google.gwt.core.client.impl\
 This came up in a separate CL -- should we just exclude *.impl.* ?  It
 seems like we would never want to consider changes to impl classes as
 API breakage.


I think this came up in the past as well. My vote is still for listing them
explicitly since the impl thing is a GWT convention, and ideally, we should
try to avoid api breakages in any public api even if it is in the impl
package. However, if someone does want to exclude particular impl packages,
adding an entry is easy.


 http://gwt-code-reviews.appspot.com/1093801/diff/1/3
 File


 tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java
 (right):

 http://gwt-code-reviews.appspot.com/1093801/diff/1/3#newcode110

 tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java:110:
 * from the most specific to the least specific.
 So if Foo(Object...) is present, Foo(null) would always resolve to
 Foo(String...)?

 yes.



 http://gwt-code-reviews.appspot.com/1093801/show


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

[gwt-contrib] Re: Ensure that all base URL's are absolut-ify-ed, no matter where we get them from. (issue1066801)

2010-11-01 Thread Amit Manjhi
LGTM

On Fri, Oct 29, 2010 at 5:41 PM, unn...@google.com wrote:

 http://gwt-code-reviews.appspot.com/1066801/show


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

[gwt-contrib] Re: Remove the onFailure call in case of unauthenticated user. This gets rid of the (issue1061801)

2010-10-28 Thread Amit Manjhi
Perhaps for non-roo apps, there could be an alternative
AuthenticationFailureHandler which simply displays a pop-up user not logged
in.

On Thu, Oct 28, 2010 at 3:49 PM, BobV b...@google.com wrote:

 On Thu, Oct 28, 2010 at 6:26 PM, Unnur Gretarsdottir unn...@google.com
 wrote:
  Bob - why is the event listener not installed by default in non roo apps?
 I
  think it was when I originally added this code (although it has been a
  while)

 There is no good default behavior in this case; authentication almost
 seems to define the phrase site-specific.  The implementation of
 AuthenticationFailureHandler makes assumptions about the serving
 environment, namely that the login and userId headers will be
 defined.  If the login header is undefined, Location.replace() is
 called with a null value.

 --
 Bob Vawter
 Google Web Toolkit Team


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

[gwt-contrib] Re: Fixed the serialization bug that the version numbers were not being sent back (issue930802)

2010-10-05 Thread Amit Manjhi
Submitted http://code.google.com/p/google-web-toolkit/source/detail?r=8932

On Mon, Oct 4, 2010 at 5:59 PM, rj...@google.com wrote:

 LGTM


 http://gwt-code-reviews.appspot.com/930802/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
The updated patch incorporates the feedback.

On Tue, Oct 5, 2010 at 11:59 AM, amitman...@google.com wrote:

 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
On Tue, Oct 5, 2010 at 1:26 PM, rj...@google.com wrote:


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001
 File

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode78
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:78:
 this.version = givenVersion;
 version = jsonObject.has(Constants.ENCODED_VERSION_PROPERTY) ?
 jsonObject.getString(Constants.ENCODED_VERSION_PROPERTY) : null;

 For that matter, do you even need the has() call? Will getString return
 null if it isn't set?


If the property is absent, getString(..) returns an JSONException.



 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1371

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1371:
 // handle DELETE
 What are you fixing by getting rid of the write operation check?

 The objective was not to get rid of the write operation check, but just to
refactor the code so that the flow is cleaner. After refactoring, a boolean
was all that was necessary.


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5002
 File


 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5002#newcode150

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:150:
 public void testEndToEndSmartDiff_NoChange() throws Exception {
 It looks like you're testing the version code path, but no longer
 testing the non-version code path? Until we delete the latter, we should
 maintain its coverage.

 Fixed in the followup patch.


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003
 File user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode200
 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:200:
 s1.isChanged = false;
 This is pointless, it defaults to false.

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode206
 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:206:
 s2.isChanged = false;
 ditto

 Done.


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode283
 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:283: *
 increment version numbers.
 Won't be able to use autobean, this is  a JRE class. Wasn't thinking.

 Document that it is only set by setUserName and the other one.

 Done.



 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
Done everything.

On Tue, Oct 5, 2010 at 1:32 PM, b...@google.com wrote:


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001
 File
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1381

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1381:
 * send an UPDATE if the client is at a version different than that of
 the
 s/s/S/

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1382

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1382:
 * server or if the server version has changed.
 or if the server version has changed after invoking the domain method.

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1385

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1385:
 assert entityInstanceAfterOperation != null;
 Unnecessary assert.

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1445

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1445:
 SerializedEntity beforeEntity = beforeDataMap.get(entityKey);
 What would a null beforeEntity imply?  That the key was just created on
 the server?


Yes, it would imply that the beforeEntity was just created on the server.



 http://gwt-code-reviews.appspot.com/959801/diff/4001/5002
 File

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5002#newcode150

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:150:
 public void testEndToEndSmartDiff_NoChange() throws Exception {
 Install eclemma and verify coverage before submitting.


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003
 File user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode283
 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:283: *
 increment version numbers.
 No plans to implement a server version of AutoBeans at present.


 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
Updated patch incorporates the feedback, including adding the additional
test.

On Tue, Oct 5, 2010 at 2:14 PM, amitman...@google.com wrote:

 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
On Tue, Oct 5, 2010 at 2:21 PM, rj...@google.com wrote:


 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001
 File
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode78
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:78:
 this.version = givenVersion;
 Not done.


Finally done.



 http://gwt-code-reviews.appspot.com/959801/diff/12002/13002

 File

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/12002/13002#newcode199

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:199:
 public void testEndToEndSmartDiff_NoChange_NoVersion() throws Exception
 {
 If I read this right, you're testing a config that has nothing to do
 with
 reality — the domain object has a version number, but the browser has
 dropped
 it. The interesting test is of a domain object that has no version
 number, or at
 least no version change. Since you have properties that don't tickle the
 version, that shouldn't be hard to write.

 Am I confused?


The test above simulated a case where the domain object has no version
number. But as you note above, there could be a scenario where the domain
object has a version number, but the client version and the server version
is the same. Added another test for that.




 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)

2010-10-05 Thread Amit Manjhi
On Tue, Oct 5, 2010 at 4:10 PM, rj...@google.com wrote:


 http://gwt-code-reviews.appspot.com/959801/diff/5004/20001

 File
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/5004/20001#newcode77
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:77:
 this.version = (Integer)
 (jsonObject.has(Constants.ENCODED_VERSION_PROPERTY)
 Why are you hard coding this requirement that the thing be an integer?


The version property is currently hard coded to be an Integer. I'm just
using that here.


 http://gwt-code-reviews.appspot.com/959801/diff/5004/20002

 File

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java
 (right):

 http://gwt-code-reviews.appspot.com/959801/diff/5004/20002#newcode200

 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:200:

 public void testEndToEndSmartDiff_NoChange_NoVersion() throws Exception
 {
 Why keep the test that tests surviving what would be a client bug?


 http://gwt-code-reviews.appspot.com/959801/show


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

[gwt-contrib] Re: Fixes to use Java 1.5 compatible JARs (issue939802)

2010-10-04 Thread Amit Manjhi
LGTM

On Mon, Oct 4, 2010 at 3:11 PM, rchan...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Fixes to use Java 1.5 compatible JARs

 Fixed DynaTableRf eclipse .classpath to use json-1.5.jar
 Fixed build scripts and eclipse classpaths to use java 1.5 compatible
 StreamHtmlParser JAR

 Review by: amitman...@google.com

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

 Affected files:
  M dev/build.xml
  M eclipse/samples/DynaTableRf/.classpath
  M eclipse/user/.classpath


 Index: dev/build.xml
 ===
 --- dev/build.xml   (revision 8929)
 +++ dev/build.xml   (working copy)
 @@ -100,7 +100,7 @@
   include name=htmlunit/htmlunit-r5940/htmlunit-r5940.jar /
   include
 name=htmlunit/htmlunit-r5940/htmlunit-core-js-r5940.jar /
   include name=nekohtml/nekohtml-1.9.13.jar /
 -  include
 name=streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar
 /
 +  include
 name=streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar
 /
   include name=xalan/xalan-2.7.1.jar /
   include name=xerces/xerces-2_9_1/serializer.jar /
   include name=xerces/xerces-2_9_1/xercesImpl-NoMetaInf.jar /
 @@ -124,7 +124,7 @@
   zipfileset src=${gwt.tools.lib}/jetty/jetty-6.1.11.jar /
   zipfileset src=${gwt.tools.lib}/icu4j/icu4j-4_4_1.jar /
   zipfileset
 src=${gwt.tools.lib}/protobuf/protobuf-2.2.0/protobuf-java-rebased-2.2.0.jar
 /
 -  zipfileset
 src=${gwt.tools.lib}/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar
 /
 +  zipfileset
 src=${gwt.tools.lib}/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar
 /
   zipfileset src=${gwt.tools.lib}/tomcat/ant-launcher-1.6.5.jar
 /
   zipfileset src=${gwt.tools.lib}/tomcat/catalina-1.0.jar /
   zipfileset
 src=${gwt.tools.lib}/tomcat/catalina-optional-1.0.jar /
 Index: eclipse/samples/DynaTableRf/.classpath
 ===
 --- eclipse/samples/DynaTableRf/.classpath  (revision 8929)
 +++ eclipse/samples/DynaTableRf/.classpath  (working copy)
 @@ -6,6 +6,6 @@
classpathentry combineaccessrules=false kind=src
 path=/gwt-user/
classpathentry kind=var
 path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA.jar
 sourcepath=/GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/
classpathentry kind=var
 path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/
 -   classpathentry kind=var
 path=GWT_TOOLS/redist/json/r2_20080312/json.jar/
 +   classpathentry kind=var
 path=GWT_TOOLS/redist/json/r2_20080312/json-1.5.jar/
classpathentry kind=output path=war/WEB-INF/classes/
  /classpath
 Index: eclipse/user/.classpath
 ===
 --- eclipse/user/.classpath (revision 8929)
 +++ eclipse/user/.classpath (working copy)
 @@ -40,6 +40,6 @@
classpathentry exported=true kind=var
 path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/
classpathentry kind=var
 path=GWT_TOOLS/lib/jetty/jetty-6.1.11.jar
 sourcepath=/GWT_TOOLS/lib/jetty/jetty-6.1.11-src.zip/
classpathentry kind=var
 path=GWT_TOOLS/lib/guava/guava-r06/guava-r06-rebased.jar/
 -   classpathentry kind=var
 path=GWT_TOOLS/lib/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar/
 +   classpathentry kind=var
 path=GWT_TOOLS/lib/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar/
classpathentry kind=output path=bin/
  /classpath




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

[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)

2010-10-01 Thread Amit Manjhi
LGTM. Other than the comments on wave that you have already looked at, I
don't have anything new. I carefully looked at the EntityProxy and stableId
stuff, and it looks great. I also skimmed over the generator code.

On Fri, Oct 1, 2010 at 5:50 PM, cromwell...@google.com wrote:


 Small nit. Besides validation of the context methods themselves, they
 also need to be matched up against the service class, I've already done
 the @ProxyFor matching part in the patch I sent, so I could do the
 @Service matching if you want.



 http://gwt-code-reviews.appspot.com/924801/diff/13001/7088
 File

 user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java
 (right):

 http://gwt-code-reviews.appspot.com/924801/diff/13001/7088#newcode288

 user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java:288:

 This validates that the return type is transportable, but does not
 validate that the parameters are transportable.


 http://gwt-code-reviews.appspot.com/924801/show


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

[gwt-contrib] Re: Use generics for EntityProxyId (issue888802)

2010-09-29 Thread Amit Manjhi
http://gwt-code-reviews.appspot.com/935801

On Wed, Sep 29, 2010 at 6:19 PM, Daniel Rice (דניאל רייס)
r...@google.comwrote:

 Somehow a class like P extends BlahBlahBlah was being treated as
 real class, resulting in a generated PImpl class which broke all
 kinds of ways.  I need to put a breakpoint at that line to see if it
 still happens and try to deal with the issue further upstream.

 Dan

 On Wed, Sep 29, 2010 at 7:58 PM,  rj...@google.com wrote:
 
  http://gwt-code-reviews.appspot.com/02/diff/1/24
  File
 
 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java
  (right):
 
  http://gwt-code-reviews.appspot.com/02/diff/1/24#newcode241
 
 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:241:
  if (P.equals(publicProxyType.getName())) {
  Uh...what?
 
  http://gwt-code-reviews.appspot.com/02/show
 


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

[gwt-contrib] Re: Removed the acquire event, renamed WriteOperation to DataEvent, renamed CREATE to PERSIST. (issue892802)

2010-09-27 Thread Amit Manjhi
Thanks for the feedback. The new patch incorporates the feedback.

On Sun, Sep 26, 2010 at 2:41 PM, rj...@google.com wrote:


 http://gwt-code-reviews.appspot.com/892802/diff/1/2
 File fixWriteOperationEnum.diff (right):

 http://gwt-code-reviews.appspot.com/892802/diff/1/2#newcode1
 fixWriteOperationEnum.diff:1: diff --git

 a/google3/third_party/java_src/gwt/svn/trunk/samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java

 b/google3/third_party/java_src/gwt/svn/trunk/samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java
 I doubt you meant to check in this diff file.

 http://gwt-code-reviews.appspot.com/892802/diff/1/4
 File user/src/com/google/gwt/app/place/AbstractProxyListActivity.java
 (right):

 http://gwt-code-reviews.appspot.com/892802/diff/1/4#newcode195
 user/src/com/google/gwt/app/place/AbstractProxyListActivity.java:195:
 update(event.getDataEvent(), event.getProxyId());
 We didn't talk about a rename for this enum, and this is a terrible
 name. Asking an event for its event?

 http://gwt-code-reviews.appspot.com/892802/diff/1/5
 File


 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
 (right):

 http://gwt-code-reviews.appspot.com/892802/diff/1/5#newcode256

 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:256:
 assert master.records.containsKey(proxyId);
 What changed here?

 http://gwt-code-reviews.appspot.com/892802/diff/1/9
 File

 user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java
 (right):

 http://gwt-code-reviews.appspot.com/892802/diff/1/9#newcode94

 user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java:94:
 if (!isFuture) {
 What's going on here? Your patch description doesn't mention fixing any
 bugs in what events are posted when, or change the semantics.

 http://gwt-code-reviews.appspot.com/892802/diff/1/15
 File


 user/test/com/google/gwt/requestfactory/client/RequestFactoryStringTest.java
 (right):

 http://gwt-code-reviews.appspot.com/892802/diff/1/15#newcode130

 user/test/com/google/gwt/requestfactory/client/RequestFactoryStringTest.java:130:
 assertEquals(1, handler.updateEventCount);
 So we do post both a persist event and an update event on create? Not
 saying that's a bad thing, just want to know and want to see it
 documented.


 http://gwt-code-reviews.appspot.com/892802/show


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

[gwt-contrib] Re: Cleanup for Roo-1401. Refactored the commit method so that it is easier for it (issue920801)

2010-09-25 Thread Amit Manjhi
Thanks. done and commited as
http://code.google.com/p/google-web-toolkit/source/detail?r=8868

On Fri, Sep 24, 2010 at 5:09 PM, rj...@google.com wrote:

 LGTM

 one name nit


 http://gwt-code-reviews.appspot.com/920801/diff/3/3001
 File


 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
 (right):

 http://gwt-code-reviews.appspot.com/920801/diff/3/3001#newcode128

 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:128:
 public void commit(JavaScriptObject returnedJso) {
 Rename to something more evocative, e.g. processFuturesAndPostEvents (if
 that's accurate)


 http://gwt-code-reviews.appspot.com/920801/show


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

[gwt-contrib] Re: Fix a corner case in getProxyId. Minor cleanup. (issue914801)

2010-09-23 Thread Amit Manjhi
Incorporates feedback.

On Thu, Sep 23, 2010 at 11:27 AM, amitman...@google.com wrote:

 http://gwt-code-reviews.appspot.com/914801/show


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

[gwt-contrib] Re: Fixed https://jira.springsource.org/browse/ROO-1429 (issue915801)

2010-09-23 Thread Amit Manjhi
updated.

On Thu, Sep 23, 2010 at 12:52 PM, amitman...@google.com wrote:

 http://gwt-code-reviews.appspot.com/915801/show


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

[gwt-contrib] Re: Re-introduces the RpcServletUtils.readContentAsUtf8() methods for backwards compatibility (issue887803)

2010-09-23 Thread Amit Manjhi
LGTM

On Thu, Sep 23, 2010 at 2:50 PM, fre...@google.com wrote:

 LGTM


 http://gwt-code-reviews.appspot.com/887803/show


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

[gwt-contrib] Re: Some Small logging cleanup (issue875802)

2010-09-22 Thread Amit Manjhi
LGTM, with the change that the testServerFailure() in RequestFactoryTest be
split into two, as we talked about in person.

On Wed, Sep 22, 2010 at 11:48 AM, unn...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Some Small logging cleanup
 - Use server error reporting in RF
 - Some documentation fixes
 - Create inherits module for projects using logging classes but not
 enabling loggin


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

 Affected files:
  A user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml
  M user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java
  M user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml
  M
 user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
  M user/src/com/google/gwt/requestfactory/server/Logging.java
  M user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java
  M user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
  M user/test/com/google/gwt/requestfactory/server/SimpleFoo.java




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

Re: [gwt-contrib] Re: Fixes two Roo issues: ROO-1238 (Amit) and ROO-1380 (pdr) (issue897801)

2010-09-20 Thread Amit Manjhi
Thanks Phil and Ray.

On Sat, Sep 18, 2010 at 12:12 PM, Ray Ryan rj...@google.com wrote:

 LGTM++


 On Sat, Sep 18, 2010 at 12:11 PM, p...@google.com wrote:

 http://gwt-code-reviews.appspot.com/897801/show


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


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

Re: [gwt-contrib] stableId

2010-09-20 Thread Amit Manjhi
stableId is not a field in the EntityProxy. It is computed when stableId()
is called on an EntityProxy. Does that help?

On Mon, Sep 20, 2010 at 10:36 AM, Patrick Julien pjul...@gmail.com wrote:

 What exactly fills in this field currently?  This field is always null
 for me on the client

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


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

[gwt-contrib] Re: Replaces the public and obnoxious String EntityProxy#getId() with the (issue902801)

2010-09-20 Thread Amit Manjhi
checkstyle fails probably because

file
name=trunk/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
error line=72 column=5 severity=error message=getEncodedId is not
alphabetical. source=com.google.gwt.checkstyle.OrderCheck/
/file

file
name=trunk/user/src/com/google/gwt/requestfactory/shared/EntityProxyId.java
error line=25 severity=error message=Type Javadoc comment is missing
an @param lt;Pgt; tag.
source=com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck/
/file

More comments coming...


On Mon, Sep 20, 2010 at 8:49 PM, rj...@google.com wrote:

 http://gwt-code-reviews.appspot.com/902801/show


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

[gwt-contrib] Re: Replaces the public and obnoxious String EntityProxy#getId() with the (issue902801)

2010-09-20 Thread Amit Manjhi
LGTM, with a TODO (amitmanjhi) to change SimpleFoo id to String and checking
if everything works.

On Mon, Sep 20, 2010 at 9:15 PM, Amit Manjhi amitman...@google.com wrote:

 checkstyle fails probably because

 file
 name=trunk/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
 error line=72 column=5 severity=error message=getEncodedId is not
 alphabetical. source=com.google.gwt.checkstyle.OrderCheck/
 /file

 file
 name=trunk/user/src/com/google/gwt/requestfactory/shared/EntityProxyId.java
 error line=25 severity=error message=Type Javadoc comment is missing
 an @param lt;Pgt; tag.
 source=com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck/
 /file

 More comments coming...


 On Mon, Sep 20, 2010 at 8:49 PM, rj...@google.com wrote:

 http://gwt-code-reviews.appspot.com/902801/show




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

Re: [gwt-contrib] Re: Remove SyncResults. Use EntityProxyId and RequestFactory.find(EntityProxyId) (issue887802)

2010-09-16 Thread Amit Manjhi
The stable id encodes the datastore id and the entity-type. If the
RequestFactory find method cannot lookup the datastoreId for a stableId, a
client side exception is thrown.

On Thu, Sep 16, 2010 at 9:23 AM, Patrick Julien pjul...@gmail.com wrote:

 On Thu, Sep 16, 2010 at 10:30 AM, Ray Ryan rj...@google.com wrote:
  For now, via the same static find methods you already provide. As quickly
 as
  we can manage it, via the same service object api already discussed.


 I'm sorry if I am slow but how is the stable id alone sufficient to
 find whatever it is you're looking for on the server?


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

[gwt-contrib] Re: JUnit ought to just serialize exceptions (issue870802)

2010-09-15 Thread Amit Manjhi
[+unnurg] who has graciously offered to review this change.

On Wed, Sep 15, 2010 at 10:15 AM, k...@google.com wrote:

 IIUC, ExceptionWrapper can be removed now after removing the last
 references in JUnitResult.


 http://gwt-code-reviews.appspot.com/870802/diff/1/3
 File user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java
 (right):

 http://gwt-code-reviews.appspot.com/870802/diff/1/3#newcode26
 user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java:26: *
 Stand-in for the non-serializable {...@link Throwable#getCause()}.
 Isn't Throwable serializable now?

 http://gwt-code-reviews.appspot.com/870802/diff/1/3#newcode44
 user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java:44:
 private transient boolean exceptionIsInitialized;
 isInitialized or isExceptionInitialized


 http://gwt-code-reviews.appspot.com/870802/show


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

[gwt-contrib] Re: Fix warnings and checkstyle errors (issue882802)

2010-09-15 Thread Amit Manjhi
LGTM

On Wed, Sep 15, 2010 at 11:17 AM, r...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Fix warnings and checkstyle errors


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

 Affected files:
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/Expenses.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesMobile.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseDetails.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseEntry.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/request/ExpenseProxy.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/employee/EmployeeDetailsView.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/employee/EmployeeEditView.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/report/ReportDetailsView.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/report/ReportEditView.java
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/server/ReportGenerator.java
  M user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java
  M user/src/com/google/gwt/app/place/AbstractProxyListView.java
  M user/src/com/google/gwt/app/place/WithTokenizers.java
  M user/src/com/google/gwt/cell/client/IconCellDecorator.java
  M user/src/com/google/gwt/editor/client/adapters/ValueBoxEditor.java
  M user/src/com/google/gwt/editor/rebind/model/EditorAccess.java
  M
 user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
  M user/src/com/google/gwt/requestfactory/client/impl/ProxyToTypeMap.java
  M user/src/com/google/gwt/requestfactory/shared/RequestEvent.java
  M user/src/com/google/gwt/requestfactory/shared/RequestObject.java
  M user/src/com/google/gwt/uibinder/elementparsers/UiChildParser.java
  M user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java
  M user/src/com/google/gwt/user/client/ui/HTML.java
  M user/src/com/google/gwt/user/client/ui/HasOneWidget.java
  M user/src/com/google/gwt/user/client/ui/IsWidget.java
  M
 user/src/com/google/gwt/validation/client/constraints/PastValidatorForDate.java
  M
 user/src/com/google/gwt/validation/client/impl/AbstractBeanDescriptor.java
  M
 user/src/com/google/gwt/validation/client/impl/ConstraintDescriptorImpl.java
  M user/src/com/google/gwt/validation/client/impl/GwtSpecificValidator.java
  M user/src/com/google/gwt/validation/client/impl/NodeImpl.java
  M user/src/com/google/gwt/validation/rebind/AbstractCreator.java
  M user/test/com/google/gwt/app/place/ActivityManagerTest.java
  M user/test/com/google/gwt/app/place/PlaceHistoryHandlerGeneratorTest.java
  M user/test/com/google/gwt/validation/client/SimpleSample.java
  M
 user/test/com/google/gwt/validation/client/constraints/DecimalMaxValidatorForNumberTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/DecimalMaxValidatorForStringTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/DecimalMinValidatorForNumberTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/DecimalMinValidatorForStringTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/DigitsValidatorForNumberTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/DigitsValidatorForStringTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/FutureValidatorForDateTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/GwtCompileTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/MaxValidatorForNumberTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/MaxValidatorForStringTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/MinValidatorForNumberTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/MinValidatorForStringTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/NotNullValidatorTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/PastValidatorForDateTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/PatternValidatorTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfBooleanTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfByteTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfCharTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfDoubleTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfFloatTest.java
  M
 user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfIntTest.java
  M
 

[gwt-contrib] Re: Remove SyncResults. Use EntityProxyId and RequestFactory.find(EntityProxyId) (issue887802)

2010-09-15 Thread Amit Manjhi
That portion has not been fixed. I was planning to get to it after this
patch. I prioritized this patch because this patch makes an api change,
where as the under-reporting is an implementation bug.

Since it is unlikely that I will get to the under-reporting bug today, I
will create another bug with a priority (major/critical) to track it. If no
one gets to it in my absence, I will finish it next week.


On Wed, Sep 15, 2010 at 8:05 PM, rj...@google.com wrote:

 Amit, https://jira.springsource.org/browse/ROO-1238 includes and fix
 under-reporting in its title, and my understanding is that this is
 addressed by your design at
 https://wave.google.com/wave/waveref/googlewave.com/w+ywx8dL_XC , in
 particular the portion where you talk about using version comparision.

 I don't see anything in this patch that touches the server side, and I
 see no use of version in the servlet.

 Have the under reporting problems been addressed yet? If they don't
 require a version comparison after all, what was the fix, in which
 patch? And why is getVersion still required by EntityProxy.

 If that issue is still not addressed, either this bug cannot be closed
 or we need a separate one addressing it.


 On 2010/09/15 22:12:12, amitmanjhi wrote:




 http://gwt-code-reviews.appspot.com/887802/show


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

[gwt-contrib] Re: Initial implementation of a general purpose Find service based on an (issue878801)

2010-09-14 Thread Amit Manjhi
Updated another patch.

On Tue, Sep 14, 2010 at 5:21 PM, rj...@google.com wrote:


 http://gwt-code-reviews.appspot.com/878801/diff/1/6
 File

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/878801/diff/1/6#newcode296

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:296:
 Long.class);
 Shouldn't you make this Object.class to keep from creating more work for
 Stephany?


That fails with Method not found since the method signature still expects
Long (in our tests, samples, etc.).


 http://gwt-code-reviews.appspot.com/878801/diff/1/8
 File user/src/com/google/gwt/requestfactory/shared/RequestFactory.java
 (right):

 http://gwt-code-reviews.appspot.com/878801/diff/1/8#newcode39
 user/src/com/google/gwt/requestfactory/shared/RequestFactory.java:39:
 FindRequest findRequest();
 What happened to making the public api

 ProxyRequestEntityProxy find(EntityProxyId proxyId)

 And putting this declaration into RFJsonImpl, along with an impl of
 find(EntityProxyId) that calls it.

 We're trying to freeze the API, no one else can swing back and clean it
 up. Last chance.


Fixed in the just uploaded patch.



 http://gwt-code-reviews.appspot.com/878801/show


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

[gwt-contrib] Re: Extract violations from being a field in sideEffects to a top-level field. Removed the handling ... (issue846802)

2010-09-13 Thread Amit Manjhi
RequestData.SIDE_EFFECTS_TOKEN is still being used at one place. Commiting
the change.

On Mon, Sep 13, 2010 at 10:03 AM, rj...@google.com wrote:

 LGTM


 http://gwt-code-reviews.appspot.com/846802/diff/1/8
 File

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (right):

 http://gwt-code-reviews.appspot.com/846802/diff/1/8#newcode672

 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:672:
 envelop.put(violations, violationsAsJson);
 If the RequestData.SIDE_EFFECTS_TOKEN constant is no longer used, please
 delete it.


 http://gwt-code-reviews.appspot.com/846802/show


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

[gwt-contrib] Re: Updates RequestFactory's Receiver type with onViolation() and onFailure() methods, making it an ... (issue855802)

2010-09-13 Thread Amit Manjhi
LGTM.

On Mon, Sep 13, 2010 at 9:48 PM, Ray Ryan rj...@google.com wrote:

 LGTM

 Your last patch looks good, and doesn't break addon-gwt


 On Mon, Sep 13, 2010 at 6:45 PM, rj...@google.com wrote:

 Oops, not quite


 http://gwt-code-reviews.appspot.com/855802/diff/10001/11003
 File user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java
 (left):

 http://gwt-code-reviews.appspot.com/855802/diff/10001/11003#oldcode127
 user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java:127:
 record = cast(syncRecord);
 Can't quite get rid of this yet, until Amit finishes the new find
 method. We need an accurate proxy to make the place change when we're
 done.


 http://gwt-code-reviews.appspot.com/855802/show




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

[gwt-contrib] RR: Record (aka Proxy) equality semantics

2010-08-30 Thread Amit Manjhi
https://wave.google.com/wave/?pli=1#restored:wave:googlewave.com%252Fw%252BOniKcQcnJ.1

The wave linked above describes how Record equality will work in GWT 2.1.
 It should be world readable, and editable by everyone
in Google-Web-Toolkit-Contributors.  Please provide feedback via separate
blips, rather than editing the main text directly.

Regards,
Amit

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

[gwt-contrib] Re: Lose idiotic, inflexible password block. (issue779803)

2010-08-26 Thread Amit Manjhi
LGTM

On Thu, Aug 26, 2010 at 2:31 PM, rj...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Lose idiotic, inflexible password block.


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

 Affected files:
  M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java


 Index:
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 ===
 --- user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (revision 8652)
 +++ user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (working copy)
 @@ -121,17 +121,7 @@

   public static final String RELATED = related;

 -  public static final SetString BLACK_LIST = initBlackList();
 -
   private static final Logger log =
 Logger.getLogger(JsonRequestProcessor.class.getName());
 -
 -  public static SetString initBlackList() {
 -SetString blackList = new HashSetString();
 -for (String str : new String[] {password}) {
 -  blackList.add(str);
 -}
 -return Collections.unmodifiableSet(blackList);
 -  }

   private RequestProperty propertyRefs;

 @@ -1047,9 +1037,9 @@
   RequestProperty propertyContext) {
 if (Record.class.isAssignableFrom(p.getType())) {
   return propertyContext.hasProperty(p.getName());
 -} else {
 -  return !BLACK_LIST.contains(p.getName());
 -}
 +}
 +
 +return true;
   }

   /**




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

[gwt-contrib] RR: design doc for ValueStore and Events

2010-08-26 Thread Amit Manjhi
https://wave.google.com/wave/#restored:search,restored:wave:googlewave.com%252Fw%252Bywx8dL_XC

The wave linked above describes how ValueStore and firing of data events
(creates, updates, and deletes) will work in GWT 2.1. It should be world
readable, and editable by everyone in Google-Web-Toolkit-Contributors. (I
just started a new wave with everyone on the RequestFactory design doc
wave.) Please provide feedback via separate blips, rather than editing the
main text directly.

Regards,
Amit

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

[gwt-contrib] Re: Fix for date serialization problem in JsonRequestProcessor. Datanucleus returns custom subtypes ... (issue790801)

2010-08-22 Thread Amit Manjhi
LGTM.

On Sun, Aug 22, 2010 at 2:07 AM, cromwell...@google.com wrote:

 Reviewers: amitmanjhi, rjrjr,

 Description:
 Fix for date serialization problem in JsonRequestProcessor. Datanucleus
 returns custom subtypes of java.util.Date from the datastore, which
 causes Dates to be serialized as english strings.


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

 Affected files:
  M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java


 Index:
 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 ===
 --- user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (revision 8616)
 +++ user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
 (working copy)
 @@ -320,7 +320,7 @@
 if (Boolean.class == type) {
   return value;
 }
 -if (Date.class == type) {
 +if (Date.class.isAssignableFrom(type)) {
   return String.valueOf(((Date) value).getTime());
 }
 if (Enum.class.isAssignableFrom(type)) {




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

[gwt-contrib] Re: Restores our new friend ValueListBox, and muzzles a couple of his (issue788801)

2010-08-20 Thread Amit Manjhi
LGTM

On Fri, Aug 20, 2010 at 1:30 AM, rj...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Restores our new friend ValueListBox, and muzzles a couple of his
 unit tests which seem to fail on FireFox. The widget works
 well manually, and M3 needs it.

 Review by: amitman...@google.com

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

 Affected files:
  M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Scaffold.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.ui.xml
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java
  A
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeRenderer.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.ui.xml
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListView.java
  M user/src/com/google/gwt/app/client/EditorSupport.java
  D user/src/com/google/gwt/app/client/ProxyBox.java
  D user/src/com/google/gwt/app/client/ProxyIdRenderer.java
  D user/src/com/google/gwt/app/client/ProxyParser.java
  D user/src/com/google/gwt/app/client/ProxyRenderer.java
  D user/src/com/google/gwt/app/client/ReadonlyProxyBox.java
  M user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
  M user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
  M user/src/com/google/gwt/app/place/AbstractRecordListView.java
  M user/src/com/google/gwt/app/place/PropertyColumn.java
  M user/src/com/google/gwt/app/place/PropertyView.java
  A user/src/com/google/gwt/app/place/ProxyRenderer.java
  M user/src/com/google/gwt/app/rebind/EditorSupportGenerator.java
  M user/src/com/google/gwt/user/client/ui/HasConstrainedValue.java
  A user/src/com/google/gwt/user/client/ui/ValueListBox.java
  M user/src/com/google/gwt/user/client/ui/ValuePicker.java
  M user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
  M user/test/com/google/gwt/user/UISuite.java
  M user/test/com/google/gwt/user/client/ui/ValueBoxBaseTest.java
  A user/test/com/google/gwt/user/client/ui/ValueListBoxTest.java




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

Re: [gwt-contrib] Re: chrome broken

2010-08-20 Thread Amit Manjhi
Can you try John's fix at http://gwt-code-reviews.appspot.com/760803/show ?
Thanks.

On Fri, Aug 20, 2010 at 12:33 PM, Pascal Patry i...@invalidip.com wrote:

 On Friday, August 20, 2010 15:31:55 John LaBanca wrote:
  We tracked it down to a Chrome dev mode oddity.  Chrome dev mode adds a
  __gwt_ObjectId attribute to all objects passed between Java and JSNI.
  I'm
  testing a simple fix now.

 Great, thank you

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


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

[gwt-contrib] Re: fix ExpenseTree for ant -f samples/expenses/build.xml build (issue781802)

2010-08-20 Thread Amit Manjhi
LGTM

On Fri, Aug 20, 2010 at 5:04 PM, k...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 fix ExpenseTree for ant -f samples/expenses/build.xml build


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

 Affected files:
  M
 samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseTree.java




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

[gwt-contrib] Re: Wrap primitive non-record types in same JSON wrapper used for Records (issue777801)

2010-08-18 Thread Amit Manjhi
Quick LGTM. I'll take another detailed look tomorrow.

On Tue, Aug 17, 2010 at 11:18 PM, cromwell...@google.com wrote:

 Reviewers: amitmanjhi, Ray Ryan,

 Description:
 Wrap primitive non-record types in same JSON wrapper used for Records


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

 Affected files:
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractBigDecimalRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractBigIntegerRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractBooleanRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractByteRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractCharacterRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractDateRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractDoubleRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractEnumRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractFloatRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractIntegerRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonListRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonObjectRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractLongRequest.java
  A
 user/src/com/google/gwt/requestfactory/client/impl/AbstractPrimitiveRequest.java
  M user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractShortRequest.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/AbstractVoidRequest.java
  M user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java
  M
 user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java
  M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java
  M user/src/com/google/gwt/requestfactory/shared/RequestData.java
  M user/src/com/google/gwt/requestfactory/shared/RequestObject.java




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

[gwt-contrib] Re: Wrap primitive non-record types in same JSON wrapper used for Records (issue777801)

2010-08-18 Thread Amit Manjhi
LGTM.

Minor nits:

   - handlePrimitiveResult(...) method can be made protected. Plus, it is
   missing @Override in all classes but AbstractPrimitiveRequest
   - the asString(..) method can be moved to AbstractPrimitiveRequest class
   and made a private member.

You would need re-sorting after the above 2 changes.

On Wed, Aug 18, 2010 at 9:42 AM, cromwell...@google.com wrote:

 http://gwt-code-reviews.appspot.com/777801/show


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

[gwt-contrib] Re: Add @Override tags in order to be able to enable Eclipse warnings without being drowned (issue754801)

2010-08-11 Thread Amit Manjhi
Does GWT mandate the eclipse setting that throws warnings for missing
@Override? I, for example, had it set to ignore.

On Wed, Aug 11, 2010 at 9:23 AM, r...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Add @Override tags in order to be able to enable Eclipse warnings
 without being drowned


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

 Affected files:
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldDetailsActivities.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListActivity.java
  M bikeshed/src/com/google/gwt/sample/expenses/server/AppCacheWarmer.java
  M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Employee.java
  M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Expense.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java
  M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Report.java
  M dev/core/src/com/google/gwt/core/ext/TreeLogger.java
  M dev/core/src/com/google/gwt/core/ext/typeinfo/JConstructor.java
  M dev/core/src/com/google/gwt/core/ext/typeinfo/JField.java
  M dev/core/src/com/google/gwt/core/ext/typeinfo/JMethod.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionAll.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionAny.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionNone.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenPropertyIs.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeAssignableTo.java
  M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeIs.java
  M dev/core/src/com/google/gwt/dev/cfg/RuleFail.java
  M dev/core/src/com/google/gwt/dev/cfg/RuleGenerateWith.java
  M dev/core/src/com/google/gwt/dev/cfg/RuleReplaceWith.java
  M dev/core/src/com/google/gwt/dev/generator/ast/BaseNode.java
  M dev/core/src/com/google/gwt/dev/generator/ast/Expression.java
  M dev/core/src/com/google/gwt/dev/javac/JsniChecker.java
  M dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java
  M dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JBooleanLiteral.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JDoubleLiteral.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JFloatLiteral.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JModVisitor.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JNode.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JNullLiteral.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JPrimitiveType.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JStringLiteral.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java
  M dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java
  M dev/core/src/com/google/gwt/dev/jjs/impl/AssertionNormalizer.java
  M dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
  M dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
  M dev/core/src/com/google/gwt/dev/jjs/impl/JsIEBlockTextTransformer.java
  M dev/core/src/com/google/gwt/dev/js/JsConstructExpressionVisitor.java
  M dev/core/src/com/google/gwt/dev/js/JsFirstExpressionVisitor.java
  M dev/core/src/com/google/gwt/dev/js/JsHoister.java
  M dev/core/src/com/google/gwt/dev/js/JsRequiresSemiVisitor.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsContinue.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsModVisitor.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsReturn.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java
  M dev/core/src/com/google/gwt/dev/js/ast/JsThrow.java
  M dev/core/src/com/google/gwt/dev/shell/EmmaStrategy.java
  M dev/core/src/com/google/gwt/dev/shell/GWTBridgeImpl.java
  M 

Re: [gwt-contrib] Re: Add @Override tags in order to be able to enable Eclipse warnings without being drowned (issue754801)

2010-08-11 Thread Amit Manjhi
LGTM

We anyway recommend the 1.5 compatibility setting.

On Wed, Aug 11, 2010 at 10:49 AM, Daniel Rice (דניאל רייס)
r...@google.comwrote:

   I've had it off but I noticed in one of the changes this morning that an
 @Override tag was (correctly) added and was curious enough to check the
 setting.  I would like to be able to enable warnings in my own Eclipse since
 there are benefits to getting it right (namely avoiding accidental
 non-overrides due to misspelling).  This CL would make it reasonable for
 those who want to to enable the warning.

 Dan


 On Wed, Aug 11, 2010 at 1:22 PM, John Tamplin j...@google.com wrote:

 On Wed, Aug 11, 2010 at 1:14 PM, Amit Manjhi amitman...@google.comwrote:

 Does GWT mandate the eclipse setting that throws warnings for missing
 @Override? I, for example, had it set to ignore.


 It is recommended and useful.

 The one catch is that if you don't tell Eclipse to use JDK1.5
 compatibility, it will insist on @Overrides on methods implementing an
 interface, which is not allowed in 1.5.

 --
 John A. Tamplin
 Software Engineer (GWT), Google




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

[gwt-contrib] Re: Another step towards making instance methods real. The interface on the client is pretty much wh... (issue710803)

2010-08-06 Thread Amit Manjhi
As we chatted over IM, the RequestDataManager class was not deleted in an
earlier patch, as it should have been. Prepared a patch that deletes the
class:

http://gwt-code-reviews.appspot.com/743801/show

On Fri, Aug 6, 2010 at 10:14 AM, cromwell...@google.com wrote:


 http://gwt-code-reviews.appspot.com/710803/diff/1/19
 File

 user/src/com/google/gwt/requestfactory/shared/impl/RequestDataManager.java
 (right):

 http://gwt-code-reviews.appspot.com/710803/diff/1/19#newcode63

 user/src/com/google/gwt/requestfactory/shared/impl/RequestDataManager.java:63:
 }
 Why was this class reinstated? This encodeParameterValue() method isn't
 being called is it? It is not correct.


 http://gwt-code-reviews.appspot.com/710803/show


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

[gwt-contrib] Re: Treat undefined Record properties on JSOs as nulls (issue744801)

2010-08-06 Thread Amit Manjhi
LGTM

On Fri, Aug 6, 2010 at 3:58 PM, cromwell...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Treat undefined Record properties on JSOs as nulls


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

 Affected files:
  M user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java
  M user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java


 Index: user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java
 ===
 --- user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java
 (revision 8500)
 +++ user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java
 (working copy)
 @@ -83,12 +83,12 @@
 // assert isDefined(property.getName()) :
 // Cannot ask for a property before setting it: 
 // + property.getName();
 -if (isNull(property.getName())) {
 +if (isNullOrUndefined(property.getName())) {
   return null;
 }
 try {
   if (Boolean.class.equals(property.getType())) {
 -  return (V) Boolean.valueOf((String) get(property.getName()));
 +  return (V) Boolean.valueOf(getBoolean(property.getName()));
   }
   if (Byte.class.equals(property.getType())) {
 return (V) Byte.valueOf((byte) getInt(property.getName()));
 @@ -189,8 +189,8 @@
   /**
* @param name
*/
 -  public final native boolean isNull(String name)/*-{
 -return this[name] === null;
 +  public final native boolean isNullOrUndefined(String name)/*-{
 +return this[name] == null;
   }-*/;

   public final boolean merge(RecordJsoImpl from) {
 @@ -299,6 +299,10 @@
 return @java.util.Date::createFrom(D)(millis);
   }-*/;

 +  private native boolean getBoolean(String name) /*-{
 +return this[name];
 +  }-*/;
 +
   private native double getDouble(String name) /*-{
 return this[name];
   }-*/;
 Index:
 user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java
 ===
 --- user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java
  (revision 8500)
 +++ user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java
  (working copy)
 @@ -122,6 +122,8 @@
 }
 assertEquals((Long) 42L, jso.getId());
 assertEquals(new Integer(1), jso.getVersion());
 +assertEquals(null, jso.get(SimpleFooRecord.longField));
 +assertEquals(null, jso.get(SimpleFooRecord.enumField));
 testSchema(jso, schemaPresent);
   }





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

[gwt-contrib] Re: Moved the create method from DeltaValueStore, used Long futureIds. Deleted ValueStore interface ... (issue721805)

2010-08-05 Thread Amit Manjhi
checked. It does not do any creates and so is fine.

On Thu, Aug 5, 2010 at 12:31 PM, rj...@google.com wrote:

 LGTM provided DynaTableRF sample is not broken


 http://gwt-code-reviews.appspot.com/721805/show


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

[gwt-contrib] Re: Fix support for null fields and Boolean type types in RequestFactory. (issue739801)

2010-08-04 Thread Amit Manjhi
imo, the client should independently check for errors, without relying on
the server code. The server code could be stale, for example.

How about if the previous code, i.e., the code before the patch, is
rearranged to check for null return value from getInt(...)?

On Wed, Aug 4, 2010 at 12:35 AM, cromwell...@google.com wrote:


 http://gwt-code-reviews.appspot.com/739801/diff/3001/1013
 File user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java
 (right):

 http://gwt-code-reviews.appspot.com/739801/diff/3001/1013#newcode131
 user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java:131:
 return null;

 Yes, but in theory this shouldn't happen since the domain object on the
 server is returning Long, and we should be aggressively checking that
 they match up at invocation time. If the domain object is returning
 String, but the Record is Long, we should be trapping this earlier.


 On 2010/08/04 07:16:38, amitmanjhi wrote:

 In addition to gracefully handling a Long field being set to null,

 wouldn't this

 return null even if a Long field is set to a String foo?


 http://gwt-code-reviews.appspot.com/739801/show


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

[gwt-contrib] Re: Forgot to commit .classpath and .project for DynaTableRf due to (issue721802)

2010-07-29 Thread Amit Manjhi
LGTM

On Thu, Jul 29, 2010 at 12:47 PM, rj...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Forgot to commit .classpath and .project for DynaTableRf due to
 .gitignore settings.

 Review by: amitman...@google.com

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

 Affected files:
  A eclipse/samples/DynaTableRf/.classpath
  A eclipse/samples/DynaTableRf/.project


 Index: eclipse/samples/DynaTableRf/.classpath
 ===
 --- eclipse/samples/DynaTableRf/.classpath  (revision 0)
 +++ eclipse/samples/DynaTableRf/.classpath  (revision 0)
 @@ -0,0 +1,8 @@
 +?xml version=1.0 encoding=UTF-8?
 +classpath
 +   classpathentry kind=src path=core/src/
 +   classpathentry kind=src output=war path=core/war/
 +   classpathentry kind=con
 path=org.eclipse.jdt.launching.JRE_CONTAINER/
 +   classpathentry combineaccessrules=false kind=src
 path=/gwt-user/
 +   classpathentry kind=output path=war/WEB-INF/classes/
 +/classpath
 Index: eclipse/samples/DynaTableRf/.project
 ===
 --- eclipse/samples/DynaTableRf/.project(revision 0)
 +++ eclipse/samples/DynaTableRf/.project(revision 0)
 @@ -0,0 +1,30 @@
 +?xml version=1.0 encoding=UTF-8?
 +projectDescription
 +   nameDynaTableRf/name
 +   comment/comment
 +   projects
 +   /projects
 +   buildSpec
 +   buildCommand
 +   nameorg.eclipse.jdt.core.javabuilder/name
 +   arguments
 +   /arguments
 +   /buildCommand
 +   buildCommand
 +
 namecom.atlassw.tools.eclipse.checkstyle.CheckstyleBuilder/name
 +   arguments
 +   /arguments
 +   /buildCommand
 +   /buildSpec
 +   natures
 +   natureorg.eclipse.jdt.core.javanature/nature
 +
 naturecom.atlassw.tools.eclipse.checkstyle.CheckstyleNature/nature
 +   /natures
 +   linkedResources
 +   link
 +   namecore/name
 +   type2/type
 +
 locationURIGWT_ROOT/samples/dynatablerf/locationURI
 +   /link
 +   /linkedResources
 +/projectDescription




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

[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)

2010-06-25 Thread Amit Manjhi
Yes, it is no longer needed. We can drop it from the maven report for the
next milestone.

On Jun 25, 2010 6:37 AM, jasonpar...@google.com wrote:
 Does this mean gwt-bikeshed.jar is no longer needed? Do we need it at
 all in the Maven repo for the next milestone?

 http://gwt-code-reviews.appspot.com/665801/show

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

[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)

2010-06-25 Thread Amit Manjhi
There is still a dist-bikeshed target that will produce the bikeshed jar.
I did not touch it. Does that cover the use case you have in mind?

This CL just ensures that the dist target not produce the bikeshed jar or
include the bikeshed jar in the distribution.

On Fri, Jun 25, 2010 at 8:01 AM, fabb...@google.com wrote:

 I have mixed feelings... if bikeshed is null now, do we think we
 will/won't need such a workspace in the future?  I thought something
 like that was being considered as the incubator replacement model, for
 example.

 I suppose we can turn it back on at will, but it seems to me that having
 build.xml always build the perhaps-empty bikeshed.  I'm less sure about
 distro-source, though... it's explicitly for in-progress work, so I'm
 not sure it's part of a release even if not empty!



 On 2010/06/25 14:23:11, amitmanjhi wrote:

 Yes, it is no longer needed. We can drop it from the maven report for

 the

 next milestone.


  On Jun 25, 2010 6:37 AM, mailto:jasonpar...@google.com wrote:
  Does this mean gwt-bikeshed.jar is no longer needed? Do we need it

 at

  all in the Maven repo for the next milestone?
 
  http://gwt-code-reviews.appspot.com/665801/show





 http://gwt-code-reviews.appspot.com/665801/show


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

[gwt-contrib] Re: Wholesale move of the app, requestfactory, and valuestore packages from the bikeshed dir to user... (issue660802)

2010-06-25 Thread Amit Manjhi
Thanks for the feedback. Since I already submitted the patch, I will create
follow-up patches for the issues below.

1. The javax validation is just used in the RequestFactory servlet. It is
currently not used by anything except the expenses sample. So I could get
away with not including it, and still not causing immediate damage.

That said, we do want to include the dependency in the distribution.
However, gwt-dev seems like the wrong place. It would be nice to include it
in gwt-servlet jar instead. What do you think?

2. I agree there is nothing bikeshed-y about the maven_script.sh. However, I
did not want to clutter of a top-level dir which just has a single script.
Move it to tools/scripts?

On Fri, Jun 25, 2010 at 8:03 AM, fabb...@google.com wrote:

 Sorry to get to this a bit late...

 I've got a couple of concerns:
  1. regarding especially the user.compile use of javax validation,
 does that suggest we should embed that library in gwt-dev.jar
 with all of our other runtime dependencies?  (I don't love that
 practice, but it has been our practice...)
  2. bikeshed/scripts/maven_script.sh seems kind of orphaned if the
 rest goes away.  There's nothing really bikeshed-y about it, is
 there?  Should it move up to a top-level scripts directory?


 On 2010/06/24 20:29:26, amitmanjhi wrote:




 http://gwt-code-reviews.appspot.com/660802/show


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

[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)

2010-06-25 Thread Amit Manjhi
Yes, the eclipse plugin code should be modified appropriately and the jar
does not need to published by maven. That is why I put you and Jason on the
reviewers list :). Can you guys take care of these issues? Thanks!

On Fri, Jun 25, 2010 at 10:58 AM, rda...@google.com wrote:

 LGTM - I'm guessing that the appropriate classes have now been merged
 into gwt-user proper?

 Side note: The plugin code can now be modified so that it doesn't have
 special handling for the bikeshed jar (i.e. putting it at the top of the
 container).


 http://gwt-code-reviews.appspot.com/665801/show


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

[gwt-contrib] Re: Moved the maven_script.sh from bikeshed/scripts to tools/scripts (issue666801)

2010-06-25 Thread Amit Manjhi
Thanks, submitted with an additional change. The script builds 'dist-dev'
instead of the now unnecessary 'dist-bikeshed' target. Verified that it
works.

On Fri, Jun 25, 2010 at 11:35 AM, Freeland Abbott fabb...@google.comwrote:

 LGTM!


 On Fri, Jun 25, 2010 at 2:31 PM, amitman...@google.com wrote:

 Reviewers: fabbott,

 Description:
 Moved the maven_script.sh from bikeshed/scripts to tools/scripts

 Patch by: amitmanjhi
 Review by: fabbott (TBR)


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

 Affected files:
  D bikeshed/scripts/maven_script.sh
  A tools/scripts/maven_script.sh


 Index: bikeshed/scripts/maven_script.sh
 ===
 --- bikeshed/scripts/maven_script.sh(revision 8316)
 +++ bikeshed/scripts/maven_script.sh(working copy)
 @@ -1,29 +0,0 @@
 -#!/bin/bash
 -#
 -# Copyright 2010 Google Inc.
 -#
 -# Licensed under the Apache License, Version 2.0 (the License); you may
 not
 -# use this file except in compliance with the License. You may obtain a
 copy of
 -# the License at
 -#
 -# http://www.apache.org/licenses/LICENSE-2.0
 -#
 -# Unless required by applicable law or agreed to in writing, software
 -# distributed under the License is distributed on an AS IS BASIS,
 WITHOUT
 -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See
 the
 -# License for the specific language governing permissions and limitations
 under
 -# the License.
 -
 -
 -# This script should be run from the trunk dir.
 -ant clean dist-bikeshed
 -
 -MAVEN_REPO=~/.m2/repository
 -
 -for i in dev user servlet
 -do
 -   mvn install:install-file -DgroupId=com.google.gwt
 -DartifactId=gwt-${i} -Dversion=2.1.0.M1 -Dpackaging=jar
 -Dfile=build/lib/gwt-${i}.jar -DgeneratePom=true
 -done
 -touch /tmp/empty-fake-soyc-vis.jar
 -mvn install:install-file -DgroupId=com.google.gwt
 -DartifactId=gwt-soyc-vis -Dversion=2.1.0.M1 -Dpackaging=jar
 -DgeneratePom=true -Dfile=/tmp/empty-fake-soyc-vis.jar
 -echo installed the gwt libs in the maven repo
 Index: tools/scripts/maven_script.sh
 ===
 --- tools/scripts/maven_script.sh   (revision 0)
 +++ tools/scripts/maven_script.sh   (revision 0)
 @@ -0,0 +1,29 @@
 +#!/bin/bash
 +#
 +# Copyright 2010 Google Inc.
 +#
 +# Licensed under the Apache License, Version 2.0 (the License); you may
 not
 +# use this file except in compliance with the License. You may obtain a
 copy of
 +# the License at
 +#
 +# http://www.apache.org/licenses/LICENSE-2.0
 +#
 +# Unless required by applicable law or agreed to in writing, software
 +# distributed under the License is distributed on an AS IS BASIS,
 WITHOUT
 +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See
 the
 +# License for the specific language governing permissions and limitations
 under
 +# the License.
 +
 +
 +# This script should be run from the trunk dir.
 +ant clean dist-bikeshed
 +
 +MAVEN_REPO=~/.m2/repository
 +
 +for i in dev user servlet
 +do
 +   mvn install:install-file -DgroupId=com.google.gwt
 -DartifactId=gwt-${i} -Dversion=2.1.0.M1 -Dpackaging=jar
 -Dfile=build/lib/gwt-${i}.jar -DgeneratePom=true
 +done
 +touch /tmp/empty-fake-soyc-vis.jar
 +mvn install:install-file -DgroupId=com.google.gwt
 -DartifactId=gwt-soyc-vis -Dversion=2.1.0.M1 -Dpackaging=jar
 -DgeneratePom=true -Dfile=/tmp/empty-fake-soyc-vis.jar
 +echo installed the gwt libs in the maven repo





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

[gwt-contrib] Re: Wholesale move of the app, requestfactory, and valuestore packages from the bikeshed dir to user... (issue660802)

2010-06-25 Thread Amit Manjhi
Okay, I created a Jira issue (https://jira.springsource.org/browse/ROO-1032)
for figuring out how to ship the javax.validation jars in the M3 timeframe.

Commited the remaining two fixes you pointed out.

On Fri, Jun 25, 2010 at 10:43 AM, Freeland Abbott fabb...@google.comwrote:

 On Fri, Jun 25, 2010 at 1:23 PM, Amit Manjhi amitman...@google.comwrote:

 Thanks for the feedback. Since I already submitted the patch, I will
 create follow-up patches for the issues below.

 1. The javax validation is just used in the RequestFactory servlet. It is
 currently not used by anything except the expenses sample. So I could get
 away with not including it, and still not causing immediate damage.

 That said, we do want to include the dependency in the distribution.
 However, gwt-dev seems like the wrong place. It would be nice to include it
 in gwt-servlet jar instead. What do you think?


 I would be squirmy about being version-married in gwt-servlet.jar; what if
 user's server code also used validation?  (We get away with it in gwt-dev,
 at least so far, but it's almost surprising to me that we do.)  I'm not sure
 how we want to address that.

 Also, I just mailed you privately, but I think SampleDataPopulator is also
 broken---it pulls classes off gwt-dev.jar (dev.util.Util), which shouldn't
 be on the server classpath.



 2. I agree there is nothing bikeshed-y about the maven_script.sh. However,
 I did not want to clutter of a top-level dir which just has a single script.
 Move it to tools/scripts?


 Works for me.



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

[gwt-contrib] Re: Introduces com.google.gwt.text from bikeshed, along with changes in (issue649801)

2010-06-18 Thread Amit Manjhi
LGTM

On Fri, Jun 18, 2010 at 3:11 PM,  rj...@google.com wrote:
 Reviewers: amitmanjhi, jgw,

 Description:
 Introduces com.google.gwt.text from bikeshed, along with changes in
 com.google.gwt.user.client.ui to take advantage of it. This is another
 step toward getting rid of the bikeshed.

 Note in particular the new TextBox-like widget ValueBox, which
 required refactoring a new abstract superclass ValueBoxBase out of
 TextBoxBase.

 A smaller change to note is the introduction of TakesValue as a
 slimmer super-interface for HasValue which doesn't require being an
 event source. The Editor framework is expected to take advantage of
 this, although that may still change a bit.

 Items that are still half-baked (IsWidget, BooleanParser,
 BooleanValueBox, etc.)  have been moved to com.google.gwt.app rather
 than text or user. This required moving some classes (e.g. the various
 Record*View interfaces) from valuestore to app, to avoid cirular
 dependencies.


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

 Affected files:
  A bikeshed/src/com/google/gwt/app/client/BooleanParser.java
  A bikeshed/src/com/google/gwt/app/client/BooleanRenderer.java
  M bikeshed/src/com/google/gwt/app/client/CellListPlacePickerView.java
  A bikeshed/src/com/google/gwt/app/client/DoubleBox.java
  A bikeshed/src/com/google/gwt/app/client/DoubleParser.java
  A bikeshed/src/com/google/gwt/app/client/DoubleRenderer.java
  M bikeshed/src/com/google/gwt/app/client/EditorSupport.java
  A bikeshed/src/com/google/gwt/app/client/IntegerBox.java
  A bikeshed/src/com/google/gwt/app/client/IntegerParser.java
  A bikeshed/src/com/google/gwt/app/client/IntegerRenderer.java
  A bikeshed/src/com/google/gwt/app/client/LongBox.java
  A bikeshed/src/com/google/gwt/app/client/LongParser.java
  A bikeshed/src/com/google/gwt/app/client/LongRenderer.java
  M bikeshed/src/com/google/gwt/app/place/AbstractRecordEditActivity.java
  M bikeshed/src/com/google/gwt/app/place/AbstractRecordListActivity.java
  A bikeshed/src/com/google/gwt/app/place/AbstractRecordListView.java
  M bikeshed/src/com/google/gwt/app/place/Activity.java
  M bikeshed/src/com/google/gwt/app/place/ActivityManager.java
  A bikeshed/src/com/google/gwt/app/place/IsWidget.java
  M bikeshed/src/com/google/gwt/app/place/PlacePicker.java
  M bikeshed/src/com/google/gwt/app/place/PlacePickerView.java
  A bikeshed/src/com/google/gwt/app/place/PropertyColumn.java
  A bikeshed/src/com/google/gwt/app/place/PropertyView.java
  A bikeshed/src/com/google/gwt/app/place/RecordDetailsView.java
  A bikeshed/src/com/google/gwt/app/place/RecordEditView.java
  A bikeshed/src/com/google/gwt/app/place/RecordListView.java
  D bikeshed/src/com/google/gwt/input/Input.gwt.xml
  D bikeshed/src/com/google/gwt/input/shared/BooleanParser.java
  D bikeshed/src/com/google/gwt/input/shared/BooleanRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/DateTimeFormatRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/DoubleParser.java
  D bikeshed/src/com/google/gwt/input/shared/DoubleRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/IntegerParser.java
  D bikeshed/src/com/google/gwt/input/shared/IntegerRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/LongParser.java
  D bikeshed/src/com/google/gwt/input/shared/LongRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/ParseException.java
  D bikeshed/src/com/google/gwt/input/shared/Parser.java
  D bikeshed/src/com/google/gwt/input/shared/PassthroughParser.java
  D bikeshed/src/com/google/gwt/input/shared/PassthroughRenderer.java
  D bikeshed/src/com/google/gwt/input/shared/Renderer.java
  M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Scaffold.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/ScaffoldListPlaceRenderer.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListActivity.java
  M
 bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListView.java
  D bikeshed/src/com/google/gwt/user/User.gwt.xml
  D 

Re: [gwt-contrib] Re: Produces the gwt-bikeshed.jar from the dist target. In GPE, the GWT (issue541801)

2010-05-17 Thread Amit Manjhi
LGTM

On Mon, May 17, 2010 at 5:06 PM,  rda...@google.com wrote:
 LGTM.

 http://gwt-code-reviews.appspot.com/541801/show

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


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


[gwt-contrib] Re: Stop trying to be clever on delete, it breaks things (issue522801)

2010-05-13 Thread Amit Manjhi
LGTM

On Thu, May 13, 2010 at 9:53 AM, rj...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Stop trying to be clever on delete, it breaks things
 Review by: amitman...@google.com

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

 Affected files:
  M
 /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java


 Index:
 /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java
 ===
 ---
 /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java
  (revision 8109)
 +++
 /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java
  (working copy)
 @@ -36,9 +36,9 @@
  * Subclasses must:
  *
  * ul
 - * liimplement a method for creating request objects
 + * liimplement methods to provide a full count, and request a specific
  * liprovide a {...@link RecordListView}
 - * lirespond to show and edit requests
 + * lirespond to show details commands
  * /ul
  *
  * Only the properties required by the view will be requested.
 @@ -147,7 +147,7 @@
 break;

   case DELETE:
 -delete(record);
 +init();
 break;

   case CREATE:
 @@ -169,13 +169,6 @@
   protected abstract void fireCountRequest(ReceiverLong callback);

   protected abstract void showDetails(R record);
 -
 -  private void delete(R record) {
 -Integer row = recordToRow.get(record.getId());
 -if (row != null) {
 -  onRangeChanged(view.asPagingListView());
 -}
 -  }

   private void getLastPage() {
 fireCountRequest(new ReceiverLong() {




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

[gwt-contrib] Re: Allow user friendly names on Property (issue509801)

2010-05-11 Thread Amit Manjhi
LGTM

On Tue, May 11, 2010 at 10:04 PM, rj...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Allow user friendly names on Property

 Review by: amitman...@google.com

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

 Affected files:
  M
 /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/EmployeeRecord.java
  M
 /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/ExpenseRecord.java
  M
 /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/ReportRecord.java
  M /bikeshed/src/com/google/gwt/valuestore/shared/Property.java
  M /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListView.java




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

[gwt-contrib] Re: Implement Delete, and make forms prettier (issue491801)

2010-05-06 Thread Amit Manjhi
Lgtm

On May 6, 2010 7:32 PM, rj...@google.com wrote:
 Verbal LGTM from Amit during desk review, submitting.

 http://gwt-code-reviews.appspot.com/491801/show

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

[gwt-contrib] Re: Re-enable user ant test for htmlunit (issue475801)

2010-05-05 Thread Amit Manjhi
LGTM. HtmlUnit should be used; it should be uncommented everywhere. I
uncommented a few things in r7787 (link below), but it seems like I missed a
few instances.

http://code.google.com/p/google-web-toolkit/source/detail?r=7787path=/trunk/user/build.xml

One nit : the assertion that ant -f user/build.xml test doesn't do
anything seems incorrect. It still runs HtmlUnit tests. No? This patch fixes
ant test.dev, ant test.web, and other similar targets.



 On Wed, May 5, 2010 at 5:52 PM, sco...@google.com wrote:

 Reviewers: amitmanjhi, jlabanca,

 Description:
 Sorry, for some reason side-by-side diff isn't working for me.

 This patch is to alleviate the fact that:

 ant -f user/build.xml test

 Does practically nothing.  Even if hmtlunit is somewhat flaky, it's
 gotten be better than nothing.

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

 Affected files:
  user/build.xml


 Index: user/build.xml
 diff --git a/user/build.xml b/user/build.xml
 index
 cac355b5497b7266b714f9ef724bb8476e448b8c..8ffde00190d4b22537009bffbee8ac2f968e9633
 100755
 --- a/user/build.xml
 +++ b/user/build.xml
 @@ -52,6 +52,7 @@
 pathelement location=${gwt.tools.lib}/easymock/easymock.jar/
 pathelement
 location=${gwt.tools.lib}/easymock/easymockclassextension.jar/
 pathelement location=${gwt.tools.lib}/objectweb/asm-3.1.jar/
 +pathelement location=${gwt.dev.jar} /
   /path

   !-- Platform shouldn't matter here, just picking one --
 @@ -97,15 +98,9 @@
 destdir=${javac.junit.out}
   classpath
 pathelement location=${javac.out} /
 -pathelement location=${gwt.build}/out/dev/bin-test /
 -pathelement
 location=${gwt.tools.lib}/tomcat/servlet-api-2.5.jar /
 pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar /
 pathelement
 location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
 -   pathelement location=${gwt.tools.lib}/cglib/cglib-2.2.jar/
 -   pathelement location=${gwt.tools.lib}/easymock/easymock.jar/
 -   pathelement
 location=${gwt.tools.lib}/easymock/easymockclassextension.jar/
 -   pathelement location=${gwt.tools.lib}/objectweb/asm-3.1.jar/
 -pathelement location=${gwt.dev.jar} /
 +path refid=test.extraclasspath /
   /classpath
 /gwt.javac
   /target
 @@ -506,9 +501,7 @@
 limit failonerror=true hours=${test.timeout}
 parallel threadsPerProcessor=${gwt.threadsPerProcessor}
 threadCount=${gwt.threadCount}
 -  !-- disable HtmlUnit until it is reliable
   antcall target=test.dev.htmlunit/
 -  --
   !-- no-op unless gwt.hosts.dev.remote is defined --
   antcall target=test.dev.remote/
   !-- no-op unless gwt.hosts.dev.selenium is defined --
 @@ -527,9 +520,7 @@
 limit failonerror=true hours=${test.timeout}
 parallel threadsPerProcessor=${gwt.threadsPerProcessor}
 threadCount=${gwt.threadCount}
 -  !-- disable HtmlUnit until it is reliable
   antcall target=test.web.htmlunit/
 -  --
   !-- no-op unless gwt.hosts.web.remote is defined --
   antcall target=test.web.remote/
   !-- no-op unless gwt.hosts.web.selenium is defined --
 @@ -548,9 +539,7 @@
 limit failonerror=true hours=${test.timeout}
 parallel threadsPerProcessor=${gwt.threadsPerProcessor}
 threadCount=${gwt.threadCount}
 -  !-- disable HtmlUnit until it is reliable
   antcall target=test.emma.htmlunit/
 -  --
   !-- no-op unless gwt.hosts.dev.remote is defined --
   antcall target=test.emma.remote/
   !-- no-op unless gwt.hosts.dev.selenium is defined --
 @@ -569,9 +558,7 @@
 limit failonerror=true hours=${test.timeout}
 parallel threadsPerProcessor=${gwt.threadsPerProcessor}
 threadCount=${gwt.threadCount}
 -  !-- disable HtmlUnit until it is reliable
   antcall target=test.draft.htmlunit/
 -  --
   !-- no-op unless gwt.hosts.web.remote is defined --
   antcall target=test.draft.remote/
   !-- no-op unless gwt.hosts.web.selenium is defined --
 @@ -590,9 +577,7 @@
 limit failonerror=true hours=${test.timeout}
 parallel threadsPerProcessor=${gwt.threadsPerProcessor}
 threadCount=${gwt.threadCount}
 -  !-- disable HtmlUnit until it is reliable
   antcall target=test.web.htmlunit/
 -  --
   !-- no-op unless gwt.hosts.web.remote is defined --
   antcall target=test.nometa.remote/
   !-- no-op unless gwt.hosts.web.selenium is defined --





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

[gwt-contrib] Re: Simplifies ActivityManager a bit, and adds a JRE unit test -- not (issue389802)

2010-04-28 Thread Amit Manjhi
Sure, I can take a look at it tomorrow morning.

On Tue, Apr 27, 2010 at 9:11 PM, rj...@google.com wrote:

 Ray, any chance of you getting to this tonight? If not, Amit, maybe you
 can take it in the morning?


 http://gwt-code-reviews.appspot.com/389802/show


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

[gwt-contrib] Re: Introduces Activity and AcitivityManager. (issue398801)

2010-04-24 Thread Amit Manjhi
Change looks great.

Btw, I took your change, fixed a few minor things and added ui.xml files for
Employee/Report details and also added the edit functionality for Report
objects. I have attached the patch relative to the original patch.


On Fri, Apr 23, 2010 at 12:05 PM, rj...@google.com wrote:

 http://gwt-code-reviews.appspot.com/398801/show


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

additionalPatch.diff
Description: Binary data


[gwt-contrib] Re: Dramatically simplifies public API of RequestFactory and (issue344804)

2010-04-24 Thread Amit Manjhi
LGTM

On Fri, Apr 16, 2010 at 3:52 PM, rj...@google.com wrote:

 Submitted after desk review w/amitmanjhi, who I'm sure will chime in
 with LGTM soon now.


 http://gwt-code-reviews.appspot.com/344804/show


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

Re: [gwt-contrib] Multiple inheritance in Widget hierarchy

2010-03-18 Thread Amit Manjhi
+1 to both Ray and Scott's answers.

On Thu, Mar 18, 2010 at 11:59 AM, Ray Ryan rj...@google.com wrote:

 It doesn't come up with HTMLUnit because from the GWT point of view
 HTMLUnit is just another browser. Prod mode is a non-issue, and for dev mode
 we have the moral equivalent of a browser plugin for it.

 On Thu, Mar 18, 2010 at 11:45 AM, Joel Webber j...@google.com wrote:

 John,

 The background behind that weird type hierarchy is that it stems from a
 time before overlay types existed. Originally c.g.g.user.client.Element was
 a simple opaque handle that had to be passed to the DOM.*() methods to get
 anything done (same for Event). After the compiler got overlay type support
 we added c.g.g.dom.client.Element as a superclass to user.Element, and
 refactored all the DOM.*() methods to use the new dom.* node/element
 hierarchy.

 Unfortunately we couldn't banish user.Element altogether without breaking
 everyone, so we ended up with a number of warts like
 setElement(user.Element). It's been on our TODO list for some time to
 deprecate user.Element and remove all references to it, but no one's found
 the time yet. As you've probably discovered, we just end up using JSO.cast()
 to work around this where necessary, but of course that won't work in real
 Java.

 If I understand what you're doing correctly, it sounds like clearing this
 up would simplify your life. But it might take a while, because we have to
 go through a fair amount of work to get there.

 @kathrin  amit: How did you guys deal with this in the HtmlUnit testing
 stuff? Sounds like it would have come up there.

 Cheers,
 joel.


 On Thu, Mar 18, 2010 at 12:45 AM, jd jdpatter...@gmail.com wrote:

 Hi,

 I am reposting this question here as the user list got no reply and I
 guess it is more a dev question:

 I am experimenting with compiling GWT code with a standard JDK so I
 can use the same code to generate HTML on both the client and the
 server.  So far it seem to be working OK but will only be practical if
 I can also get UIBinder and i18n working.

 My goal is to create HTML pages that can be crawled and indexed and
 also allow GWT code to add, load and modify the page.  Others have
 recommended building two parallel sites - an html one and a GT one
 which seems a bit redundant.

 My experiement has put a real w3c Node inside every GWT Node and
 replace native methods with ones that manipulate the w3c node.  Then
 finally I take the full w3c node from any element and convert it into
 html.

 I found that the object hierarchy needed to be changed to be valid
 Java.  An example of the issue is with the Anchor widget:

  public Anchor() {
setElement(Document.get().createAnchorElement());
setStyleName(gwt-Anchor);
  }

 com.google.gwt.dom.client.AnchorElement extends
 com.google.gwt.dom.client.AnchorElement but setElement expects a
 com.google.gwt.user.client.Element so AnchorElement must extend both
 classes which is impossible.


 I have modified AnchorElement and friends to extends
 com.google.gwt.user.client.Element instead which seems to have
 worked.


 My question is:  Is this impossible inheritance hierarchy intentional
 to stop this kind of messing about?

 Cheers,

 John

 --
 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: Introduces expenses scaffolding app, ExpensesScaffold, complete with

2010-03-17 Thread Amit Manjhi
Followup question.

- Is Expenses.gwt.xml necessary with the introduction of
ExpensesScaffolding.xml? Can the Expenses.java functionality be moved into
ExpensesScaffolding.java?


On Wed, Mar 17, 2010 at 11:14 AM, amitman...@google.com wrote:

 Additional checkstyle problems (Eclipse output). Please run checkstyle
 :)

 Description ResourcePathLocationType
 forProperties is not
 alphabetical.   EntityListRequest.java
  Bikeshed/src/com/google/gwt/requestfactory/shared   line
 35  Checkstyle Problem
 getDetailsPlaceFor is not
 alphabetical.   Places.java
 Bikeshed/src/com/google/gwt/sample/expenses/client/placeline
 34  Checkstyle Problem
 Line does not match expected header line of
 '/\*'.  ExpensesScaffold.java
 Bikeshed/src/com/google/gwt/sample/expenses/client  line
 1   Checkstyle Problem
 Line does not match expected header line of
 '/\*'.  ListRequester.java
  Bikeshed/src/com/google/gwt/sample/expenses/client  line
 1   Checkstyle Problem
 Line matches the illegal pattern 'newline before
 }'. ListRequester.java
  Bikeshed/src/com/google/gwt/sample/expenses/client  line
 45  Checkstyle Problem


 http://gwt-code-reviews.appspot.com/209802/diff/1/16
 File


 bikeshed/src/com/google/gwt/sample/expenses/client/ExpensesScaffoldShell.java
 (right):

 http://gwt-code-reviews.appspot.com/209802/diff/1/16#newcode32
 Line 32: interface Binder extends UiBinderWidget,
 ExpensesScaffoldShell {}
 checkstyle. { is not followed by whitespace.

 http://gwt-code-reviews.appspot.com/209802/diff/1/24
 File

 bikeshed/src/com/google/gwt/sample/expenses/client/place/EntityPlace.java
 (right):

 http://gwt-code-reviews.appspot.com/209802/diff/1/24#newcode45
 Line 45: if (this == obj)
 checkstyle: not alphabetical

 http://gwt-code-reviews.appspot.com/209802/diff/1/34
 File

 bikeshed/src/com/google/gwt/sample/expenses/server/domain/Storage.java
 (right):

 http://gwt-code-reviews.appspot.com/209802/diff/1/34#newcode145
 Line 145: synchronized ListReport findAllReports() {
 checkstyle: findAllReports is not alphabetical

 http://gwt-code-reviews.appspot.com/209802/diff/1/42
 File bikeshed/src/com/google/gwt/user/client/ui/ValueListBox.java
 (right):

 http://gwt-code-reviews.appspot.com/209802/diff/1/42#newcode87
 Line 87: public HandlerRegistration
 addValueChangeHandler(ValueChangeHandlerT handler) {
 checkstyle: method is not alphabetical.


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


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

[gwt-contrib] Re: allow skipping unit tests in development or production mode

2010-03-11 Thread Amit Manjhi
yeah the implementation looks fine

On Mar 11, 2010 8:32 AM, Lex Spoon sp...@google.com wrote:

On Wed, Mar 10, 2010 at 7:17 PM, amitman...@google.com wrote:

 LGTM on this quick and dirty sol...
I feel the same way.  A quick and dirty solution would be very valuable so
that I can add tests for the cross-site linker.  However, it looks like we
ultimately want to support an or of a bunch of ands.

Did you review the implementation, Amit?  So far everyone is okay with the
API as something to work with for now.

Lex

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

[gwt-contrib] Re: Refactor SessionHandler and BrowserChannelClient to allow other OOPHM clients than HtmlUnit

2009-11-28 Thread Amit Manjhi
Thanks for the patch. I agree with John that this is way too late for 2.0 at
this point. I will get back to this for 2.1. Please file a public issue for
this refactoring and attach the patch to that.

Regards,
Amit

On Sat, Nov 28, 2009 at 6:41 PM, j...@google.com wrote:

 I really think this is too big a change to make between the RC and the
 final release.  I would rather wait until after we ship 2.0 and spend a
 bit more time on it to clean up some of the other rough edges before
 getting it into trunk.



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


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

Re: [gwt-contrib] Remove distro-source/platform dirs

2009-11-20 Thread Amit Manjhi
ping

On Thu, Nov 19, 2009 at 12:04 PM, Amit Manjhi amitman...@google.com wrote:

 [Somehow, I got an error from the mail delivery system the first time.
 Apologies if you have already received it]

 Description:
 Remove distro-source/windows, distro-source/linux and distro-source/mac
 dirs and their contents.

 There is a libswt-webkit-carbon-3235.jnilib that looks safe to remove,
 since I could not find any references to it. I just wanted to confirm
 before removing it.

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

 Affected files:
   distro-source/linux/src/mozilla-hosted-browser.conf
   distro-source/mac/src/libswt-webkit-carbon-3235.jnilib


 Index: distro-source/linux/src/mozilla-hosted-browser.conf
 ===
 --- distro-source/linux/src/mozilla-hosted-browser.conf (revision 7023)
 +++ distro-source/linux/src/mozilla-hosted-browser.conf (working copy)
 @@ -1,35 +0,0 @@
 -# This file specifies a list of possible mozilla installations to load, in
 -# priority order.  Non-existent paths are ignored, and the next location
 is
 -# searched.  A best effort attempt is also made to ignore paths containing
 -# incompatible mozilla installations (for example, ones which use GTK1
 instead
 -# of GTK2, which is required).  The first apparently valid installation
 found
 -# will be used (although it could still fail later).
 -#
 -# Non-absolute paths are relative to the directory containing this file
 (that
 -# is, the GWT install directory).
 -#
 -# Non-system installations should contain a file
 named gwt-dl-loadorder.conf.
 -# The format of such a file is one shared library per line which dictates
 the
 -# order in which that installation's shared libraries must be loaded to
 prevent
 -# implicit loading of other libraries.  In other words, no library should
 be
 -# loaded before its dependencies.  This is to prevent the LD_LIBRARY_PATH
 or
 -# other system library load configuration from loading the default system
 -# version of the library instead of the version in the target
 installation.
 -
 -
 -# Prefer mozilla 1.7.13 if it exists, because it supports mouse wheel
 events.
 -# If you need mouse wheel events, you can install the distribution
 available at:
 -#
 -#

 http://google-web-toolkit.googlecode.com/svn/tools/redist/mozilla/mozilla-1.7.13.tar.gz
 -#
 -# However, this version may not run correctly on your system.  If it
 doesn't,
 -# you can try installing a mozilla 1.7.13 built for your system.
 -mozilla-1.7.13
 -
 -# This is the default mozilla that ships with GWT.
 -mozilla-1.7.12
 -
 -# See if there are compatible mozilla distributions already installed.
 -/usr/lib/mozilla-1.7.13
 -/usr/lib/mozilla-1.7.12
 -/usr/lib/mozilla
 Index: distro-source/mac/src/libswt-webkit-carbon-3235.jnilib
 ===
 Cannot display: file marked as a binary type.
 svn:mime-type = application/octet-stream


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




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

[gwt-contrib] Re: Remove distro-source/platform dirs

2009-11-20 Thread Amit Manjhi
Kelly and Scott are the only ones that have touched this file. Please
comment if it is still necessary.

On Fri, Nov 20, 2009 at 2:41 PM, j...@google.com wrote:

 mozilla-hosted-browser.conf is definitely not needed any more.  I don't
 believe libswt is needed for Mac either, but then I am not sure why it
 was ever here in the first place rather than in jni/mac.



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


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

[gwt-contrib] Re: Remove distro-source/platform dirs

2009-11-20 Thread Amit Manjhi
Thanks! commited at r7085 (trunk) and r7086 (2.0)

On Fri, Nov 20, 2009 at 2:52 PM, Scott Blum sco...@google.com wrote:

 Strange, I'd have thought we'd have pulled that file from /tools.  Anyway,
 please kill it.


 On Fri, Nov 20, 2009 at 5:48 PM, Amit Manjhi amitman...@google.comwrote:

 Kelly and Scott are the only ones that have touched this file. Please
 comment if it is still necessary.


 On Fri, Nov 20, 2009 at 2:41 PM, j...@google.com wrote:

 mozilla-hosted-browser.conf is definitely not needed any more.  I don't
 believe libswt is needed for Mac either, but then I am not sure why it
 was ever here in the first place rather than in jni/mac.



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





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

[gwt-contrib] Remove distro-source/platform dirs

2009-11-19 Thread Amit Manjhi
[Somehow, I got an error from the mail delivery system the first time.
Apologies if you have already received it]

Description:
Remove distro-source/windows, distro-source/linux and distro-source/mac
dirs and their contents.

There is a libswt-webkit-carbon-3235.jnilib that looks safe to remove,
since I could not find any references to it. I just wanted to confirm
before removing it.

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

Affected files:
  distro-source/linux/src/mozilla-hosted-browser.conf
  distro-source/mac/src/libswt-webkit-carbon-3235.jnilib


Index: distro-source/linux/src/mozilla-hosted-browser.conf
===
--- distro-source/linux/src/mozilla-hosted-browser.conf (revision 7023)
+++ distro-source/linux/src/mozilla-hosted-browser.conf (working copy)
@@ -1,35 +0,0 @@
-# This file specifies a list of possible mozilla installations to load, in
-# priority order.  Non-existent paths are ignored, and the next location is
-# searched.  A best effort attempt is also made to ignore paths containing
-# incompatible mozilla installations (for example, ones which use GTK1
instead
-# of GTK2, which is required).  The first apparently valid installation
found
-# will be used (although it could still fail later).
-#
-# Non-absolute paths are relative to the directory containing this file
(that
-# is, the GWT install directory).
-#
-# Non-system installations should contain a file
named gwt-dl-loadorder.conf.
-# The format of such a file is one shared library per line which dictates
the
-# order in which that installation's shared libraries must be loaded to
prevent
-# implicit loading of other libraries.  In other words, no library should
be
-# loaded before its dependencies.  This is to prevent the LD_LIBRARY_PATH
or
-# other system library load configuration from loading the default system
-# version of the library instead of the version in the target installation.
-
-
-# Prefer mozilla 1.7.13 if it exists, because it supports mouse wheel
events.
-# If you need mouse wheel events, you can install the distribution
available at:
-#
-#
http://google-web-toolkit.googlecode.com/svn/tools/redist/mozilla/mozilla-1.7.13.tar.gz
-#
-# However, this version may not run correctly on your system.  If it
doesn't,
-# you can try installing a mozilla 1.7.13 built for your system.
-mozilla-1.7.13
-
-# This is the default mozilla that ships with GWT.
-mozilla-1.7.12
-
-# See if there are compatible mozilla distributions already installed.
-/usr/lib/mozilla-1.7.13
-/usr/lib/mozilla-1.7.12
-/usr/lib/mozilla
Index: distro-source/mac/src/libswt-webkit-carbon-3235.jnilib
===
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream


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

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

[gwt-contrib] Re: Remove startOnFirstThread references from webAppCreator

2009-11-18 Thread Amit Manjhi
I tested it on Mac and on Linux.

On Wed, Nov 18, 2009 at 11:28 AM, j...@google.com wrote:

 LGTM, but I haven't tested it


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


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

[gwt-contrib] Re: Remove startOnFirstThread references from webAppCreator

2009-11-18 Thread Amit Manjhi
thanks! commited as r6993 (trunk) and r6994 (2.0).

On Wed, Nov 18, 2009 at 11:31 AM, Amit Manjhi amitman...@google.com wrote:

 I tested it on Mac and on Linux.


 On Wed, Nov 18, 2009 at 11:28 AM, j...@google.com wrote:

 LGTM, but I haven't tested it


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




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

[gwt-contrib] Re: Fix thread safety issues in GwtLocaleFactoryImpl and LocaleUtils

2009-11-16 Thread Amit Manjhi

 http://gwt-code-reviews.appspot.com/102819/diff/1/7#newcode80
 Line 80:
 On 2009/11/16 19:51:53, amitmanjhi wrote:

 use ConcurrentHashMap? Can avoid the lock.


 I don't believe so, as you have to hold the lock across
 containsKey/get/put.


Wouldn't putIfAbsent(..) work?

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

[gwt-contrib] Re: Fix thread safety issues in GwtLocaleFactoryImpl and LocaleUtils

2009-11-16 Thread Amit Manjhi
LGTM

On Mon, Nov 16, 2009 at 1:19 PM, John Tamplin j...@google.com wrote:

 On Mon, Nov 16, 2009 at 3:10 PM, Amit Manjhi amitman...@google.comwrote:

  use ConcurrentHashMap? Can avoid the lock.


 I don't believe so, as you have to hold the lock across
 containsKey/get/put.


 Wouldn't putIfAbsent(..) work?


 Since gets never block, it seems like this can't be used to ensure that all
 threads come up with the same GwtLocale instance for a given locale.

 The current implementation seems clearly correct, and the fact that we are
 having this discussion indicates using ConcurrentHashMap is not a clear win,
 especially given the time constraints.

 --
 John A. Tamplin
 Software Engineer (GWT), Google


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

[gwt-contrib] Re: RR: Introduces system property gwt.testMethodTimeoutMinutes

2009-11-03 Thread Amit Manjhi
I agree with John. A system property is a global VM-wide option that seems
like an overkill for this purpose. A command line option with sane defaults
is probably the way to go.

On Tue, Nov 3, 2009 at 3:50 PM, j...@google.com wrote:

 On 2009/11/03 23:49:14, Ray Ryan wrote:

 I imagine I'm not the only one who tweaks

 JUnitShell.TEST_METHOD_TIMEOUT_MILLIS

 when in the middle of long debug sessions. Wouldn't it be nice to be

 able to do

 so without touching the code?


  General questions: is a system property the right way to do this; is

 the

 property name okay; and is it okay to hit the system properties at

 class init

 time like that?


  Also deletes unneeded @SuppressWarnings


 I think this would be better as a JUnitShell command line option.


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


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



[gwt-contrib] Re: Run tests multiple times with HtmlUnit to reduce false positives of failures

2009-11-02 Thread Amit Manjhi
Yeah, the formatting is incorrect. The default formatting on my eclipse
seems incorrect.

I will upload a re-formatted patch, after fixing the eclipse issue. Thanks
John.

On Mon, Nov 2, 2009 at 2:09 PM, j...@google.com wrote:


 http://gwt-code-reviews.appspot.com/91806/diff/1/3
 File user/src/com/google/gwt/junit/JUnitShell.java (right):

 http://gwt-code-reviews.appspot.com/91806/diff/1/3#newcode698
 Line 698: getTopLogger().log(TreeLogger.ERROR,
 This formatting change doesn't look right (the previous version fit fine
 in 80 characters) and is unrelated to the actual change being made.

 http://gwt-code-reviews.appspot.com/91806/diff/1/4
 File user/src/com/google/gwt/junit/RunStyle.java (right):

 http://gwt-code-reviews.appspot.com/91806/diff/1/4#newcode78
 Line 78: * an error setting up for that mode
 These changes aren't related and I think are actually incorrect, since
 continuation lines should be indented 4 characters.


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


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



[gwt-contrib] Re: HostedMode relay to DevMode

2009-10-15 Thread Amit Manjhi
LGTM, with a minor update to the WARN message, as we discussed on IM.
Final text of the WARN message:
The class com.google.gwt.dev.HostedMode is deprecated and will be removed
-- use com.google.gwt.dev.DevMode instead for launching development mode

On Thu, Oct 15, 2009 at 11:40 AM, j...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 This is the class which will take the place of the existing HostedMode
 entry point (which will be renamed to DevMode).

 All it does is log a warning and run the real DevMode main.

 As noted in the code, this is suitable for testing in current trunk but
 things will be renamed before submit.

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

 Affected files:
  dev/core/src/com/google/gwt/dev/OldHostedMode.java


 Index: dev/core/src/com/google/gwt/dev/OldHostedMode.java
 ===
 --- dev/core/src/com/google/gwt/dev/OldHostedMode.java  (revision 0)
 +++ dev/core/src/com/google/gwt/dev/OldHostedMode.java  (revision 0)
 @@ -0,0 +1,56 @@
 +/*
 + * Copyright 2009 Google Inc.
 + *
 + * Licensed under the Apache License, Version 2.0 (the License); you may
 not
 + * use this file except in compliance with the License. You may obtain a
 copy of
 + * the License at
 + *
 + * http://www.apache.org/licenses/LICENSE-2.0
 + *
 + * Unless required by applicable law or agreed to in writing, software
 + * distributed under the License is distributed on an AS IS BASIS,
 WITHOUT
 + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See
 the
 + * License for the specific language governing permissions and limitations
 under
 + * the License.
 + */
 +package com.google.gwt.dev;
 +
 +import com.google.gwt.core.ext.TreeLogger;
 +
 +/**
 + * Support old name for this entry point, logging a warning message before
 + * redirecting to the new name.
 + *
 + * TODO(jat): after trunk merge, rename this to HostedMode and HostedMode
 to
 + * DevMode.
 + */
 +...@deprecated
 +public class OldHostedMode extends HostedMode {
 +
 +  /**
 +   * Support old name for this entry point, logging a warning message
 before
 +   * redirecting to the new name.
 +   *
 +   * @param args command-line arguments
 +   */
 +  public static void main(String[] args) {
 +OldHostedMode hostedMode = new OldHostedMode();
 +if (new ArgProcessor(hostedMode.options).processArgs(args)) {
 +  hostedMode.run();
 +  // Exit w/ success code.
 +  System.exit(0);
 +}
 +// Exit w/ non-success code.
 +System.exit(-1);
 +  }
 +
 +  @Override
 +  protected boolean doStartup() {
 +if (!super.doStartup()) {
 +  return false;
 +}
 +getTopLogger().log(TreeLogger.WARN,
 +HostedMode is deprecated -- use DevMode instead);
 +return true;
 +  }
 +}

 Property changes on: dev/core/src/com/google/gwt/dev/OldHostedMode.java
 ___
 Added: svn:mime-type
   + text/x-java
 Added: svn:eol-style
   + native





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



[gwt-contrib] Re: Add a HelpInfo link for old plugins

2009-10-14 Thread Amit Manjhi
LGTM

On Wed, Oct 14, 2009 at 12:05 PM, j...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 A warning is printed when a really old plugin connects to the code
 server.  This adds a HelpInfo link, which points at the UsingOOPHM web
 page for now, giving users information about getting a later plugin.

 In the future, this will be changed to a better location.

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

 Affected files:
  dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java


 Index: dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java
 ===
 --- dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java
  (revision 6372)
 +++ dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java
  (working copy)
 @@ -16,12 +16,15 @@
  package com.google.gwt.dev.shell;

  import com.google.gwt.core.ext.TreeLogger;
 +import com.google.gwt.core.ext.TreeLogger.HelpInfo;
  import com.google.gwt.dev.shell.JsValue.DispatchObject;

  import java.io.IOException;
  import java.io.InputStream;
  import java.io.OutputStream;
 +import java.net.MalformedURLException;
  import java.net.Socket;
 +import java.net.URL;
  import java.util.HashMap;
  import java.util.Map;
  import java.util.Set;
 @@ -233,8 +236,27 @@
 moduleName = oldLoadModule.getModuleName();
 userAgent = oldLoadModule.getUserAgent();
 protocolVersion = 1;
 +HelpInfo helpInfo = new HelpInfo() {
 +  @Override
 +  public String getAnchorText() {
 +return UsingOOPHM wiki page;
 +  }
 +
 +  @Override
 +  public URL getURL() {
 +try {
 +  // TODO(jat): better landing page for more info
 +  return new URL(
 +  
 http://code.google.com/p/google-web-toolkit/wiki/UsingOOPHM;);
 +} catch (MalformedURLException e) {
 +  // can't happen
 +  return null;
 +}
 +  }
 +};
 logger.log(TreeLogger.WARN, Connection from old browser plugin --
 
 -+ please upgrade to a later version for full functionality);
 ++ please upgrade to a later version for full functionality,
 null,
 +helpInfo);
 break;
   case CHECK_VERSIONS:
 String connectError = null;




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



[gwt-contrib] Re: Remove @DoNotRunWith annotation of the passed test cases

2009-10-13 Thread Amit Manjhi
Hi Frank,

The farewellSwt branch is no longer active. John merged it back in trunk in
r6366. Can you update this patch to be against trunk, post r6366? Also, can
you double-check if the comments for tests that no longer use @DoNotRunWith
need to be updated?

Thanks,
Amit

On Tue, Oct 13, 2009 at 5:47 PM, f...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 @DoNotRunWith({Platform.Htmlunit}) is the one gating the Htmlunit test
 cases that failed. Since bug fixes in GWT and HtmlUnit, some of these
 annotated test case are already passing. Need to remove these
 annotations.

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

 Affected files:
  user/test/com/google/gwt/dev/jjs/test/RunAsyncMetricsIntegrationTest.java
  user/test/com/google/gwt/dom/client/ElementTest.java
  user/test/com/google/gwt/dom/client/MapTests.java
  user/test/com/google/gwt/dom/client/NodeTest.java
  user/test/com/google/gwt/event/dom/client/DomEventTest.java
  user/test/com/google/gwt/uibinder/sample/client/UiBinderTest.java
  user/test/com/google/gwt/user/client/rpc/UnicodeEscapingTest.java
  user/test/com/google/gwt/user/client/ui/AnchorTest.java
  user/test/com/google/gwt/user/client/ui/ButtonTest.java
  user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
  user/test/com/google/gwt/user/client/ui/CreateEventTest.java
  user/test/com/google/gwt/user/client/ui/DOMTest.java
  user/test/com/google/gwt/user/client/ui/DialogBoxTest.java
  user/test/com/google/gwt/user/client/ui/HistoryTest.java
  user/test/com/google/gwt/user/client/ui/ImageTest.java
  user/test/com/google/gwt/user/client/ui/MenuBarTest.java
  user/test/com/google/gwt/user/client/ui/PopupTest.java
  user/test/com/google/gwt/user/client/ui/SimpleRadioButtonTest.java
  user/test/com/google/gwt/user/client/ui/SuggestBoxTest.java
  user/test/com/google/gwt/user/client/ui/TreeTest.java
  user/test/com/google/gwt/xml/client/XMLTest.java




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



Announcing GWT 2.0 Milestone 1

2009-10-05 Thread Amit Manjhi

Hi everyone,

We are excited to release the first milestone build for GWT 2.0 today.
This milestone provides early access (read: known to still be
unfinished and buggy) to the various bits of core functionality that
will be coming in GWT 2.0. Please download the bits from:

http://code.google.com/p/google-web-toolkit/downloads/list?can=1q=2.0+Milestone+1


Things that are changing with GWT 2.0 that might otherwise be
confusing without explanation
* Terminology changes: We're going to start using the term
development mode rather than the old term hosted mode. The term
hosted mode was sometimes confusing to people, so we'll be using the
more descriptive term from now on. For similar reasons, we'll be using
the term production mode rather than web mode when referring to
compiled script.

* Changes to the distribution: Note that there's only one download,
and it's no longer platform-specific. You download the same zip file
for every development platform. This is made possible by the new
plugin approach used to implement development mode (see below). The
distribution file does not include the browser plugins themselves;
those are downloaded separately the first time you use development
mode in a browser that doesn't have the plugin installed.


Functionality that will be coming in GWT 2.0
* In-Browser Development Mode: Prior to 2.0, GWT hosted mode provided
a special-purpose hosted browser to debug your GWT code. In 2.0, the
web page being debugged is viewed within a regular-old browser.
Development mode is supported through the use of a native-code plugin
for each browser. In other words, you can use development mode
directly from Safari, Firefox, IE, and Chrome.

* Code Splitting: Developer-guided code splitting allows you to chunk
your GWT code into multiple fragments for faster startup. Imagine
having to download a whole movie before being able to watch it. Well,
that's what you have to do with most Ajax apps these days -- download
the whole thing before using it. With code splitting, you can arrange
to load just the minimum script needed to get the application running
and the user interacting, while the rest of the app is downloaded as
needed.

* Declarative User Interface: GWT's UiBinder now allows you to create
user interfaces mostly declaratively. Previously, widgets had to be
created and assembled programmatically, requiring lots of code. Now,
you can use XML to declare your UI, making the code more readable,
easier to maintain, and faster to develop. The Mail sample has been
updated to use the new declarative UI.

* Bundling of resources (ClientBundle): GWT has shipped with
ImageBundles since GWT v1.4, giving developers automatic spriting of
images. ClientBundle generalizes this technique, bringing the power of
combining and optimizing resources into one download to things like
text files, CSS, and XML. This means fewer network round trips, which
in turn can decrease application latency -- especially on mobile
applications.

* Using HtmlUnit for running GWT tests: GWT 2.0 no longer uses SWT or
the old mozilla code (on linux) to run GWT tests. Instead, it uses
HtmlUnit as the built-in browser. HtmlUnit is 100% Java. This means
there is a single GWT distribution for linux, mac, and windows, and
debugging GWT Tests in development mode can be done entirely in a Java
debugger.


Known issues
*  If you are planning to run the webAppCreator, i18nCreator, or the
junitCreator scripts on Mac or Linux, please set their executable bits
by doing a 'chmod +x *Creator'
* Our HtmlUnit integration is still not complete. Additionally,
HtmlUnit does not do layout. So tests can fail either because they
exercise layout or they hit bugs due to incomplete integration. If you
want such tests to be ignored on HtmlUnit, please annotate the test
methods with @DoNotRunWith({Platform.Htmlunit})
* The Google Eclipse Plugin will only allow you to add GWT release
directories that include a file with a name like gwt-dev-windows.jar.
You can fool it by sym linking or copying gwt-dev.jar to the
appropriate name.


Breaking changes
* The way arguments are passed to the GWT testing infrastructure has
been revamped. There is now a consistent syntax to support arbitrary
runstyles, including user-written with no changes to GWT.  Though
this does not affect common launch configs, some of the less common
ones will need to be updated. For example, '-selenium FF3' has become
'-runStyle selenium:FF3'


As always, remember that GWT milestone builds like this are use-at-
your-own-risk and we don't recommend it for production use. Please
report any bugs you encounter to the GWT issue tracker (http://
code.google.com/p/google-web-toolkit/issues/list) after doing a quick
search to see if your issue has already been reported.

-- Amit Manjhi, on behalf of the Google Web Toolkit team
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
Google Web Toolkit group.
To post

[gwt-contrib] Announcing GWT 2.0 Milestone 1

2009-10-05 Thread Amit Manjhi

Hi everyone,

We are excited to release the first milestone build for GWT 2.0 today.
This milestone provides early access (read: known to still be
unfinished and buggy) to the various bits of core functionality that
will be coming in GWT 2.0. Please download the bits from:

http://code.google.com/p/google-web-toolkit/downloads/list?can=1q=2.0+Milestone+1


Things that are changing with GWT 2.0 that might otherwise be
confusing without explanation
* Terminology changes: We're going to start using the term
development mode rather than the old term hosted mode. The term
hosted mode was sometimes confusing to people, so we'll be using the
more descriptive term from now on. For similar reasons, we'll be using
the term production mode rather than web mode when referring to
compiled script.

* Changes to the distribution: Note that there's only one download,
and it's no longer platform-specific. You download the same zip file
for every development platform. This is made possible by the new
plugin approach used to implement development mode (see below). The
distribution file does not include the browser plugins themselves;
those are downloaded separately the first time you use development
mode in a browser that doesn't have the plugin installed.


Functionality that will be coming in GWT 2.0
* In-Browser Development Mode: Prior to 2.0, GWT hosted mode provided
a special-purpose hosted browser to debug your GWT code. In 2.0, the
web page being debugged is viewed within a regular-old browser.
Development mode is supported through the use of a native-code plugin
for each browser. In other words, you can use development mode
directly from Safari, Firefox, IE, and Chrome.

* Code Splitting: Developer-guided code splitting allows you to chunk
your GWT code into multiple fragments for faster startup. Imagine
having to download a whole movie before being able to watch it. Well,
that's what you have to do with most Ajax apps these days -- download
the whole thing before using it. With code splitting, you can arrange
to load just the minimum script needed to get the application running
and the user interacting, while the rest of the app is downloaded as
needed.

* Declarative User Interface: GWT's UiBinder now allows you to create
user interfaces mostly declaratively. Previously, widgets had to be
created and assembled programmatically, requiring lots of code. Now,
you can use XML to declare your UI, making the code more readable,
easier to maintain, and faster to develop. The Mail sample has been
updated to use the new declarative UI.

* Bundling of resources (ClientBundle): GWT has shipped with
ImageBundles since GWT v1.4, giving developers automatic spriting of
images. ClientBundle generalizes this technique, bringing the power of
combining and optimizing resources into one download to things like
text files, CSS, and XML. This means fewer network round trips, which
in turn can decrease application latency -- especially on mobile
applications.

* Using HtmlUnit for running GWT tests: GWT 2.0 no longer uses SWT or
the old mozilla code (on linux) to run GWT tests. Instead, it uses
HtmlUnit as the built-in browser. HtmlUnit is 100% Java. This means
there is a single GWT distribution for linux, mac, and windows, and
debugging GWT Tests in development mode can be done entirely in a Java
debugger.


Known issues
*  If you are planning to run the webAppCreator, i18nCreator, or the
junitCreator scripts on Mac or Linux, please set their executable bits
by doing a 'chmod +x *Creator'
* Our HtmlUnit integration is still not complete. Additionally,
HtmlUnit does not do layout. So tests can fail either because they
exercise layout or they hit bugs due to incomplete integration. If you
want such tests to be ignored on HtmlUnit, please annotate the test
methods with @DoNotRunWith({Platform.Htmlunit})
* The Google Eclipse Plugin will only allow you to add GWT release
directories that include a file with a name like gwt-dev-windows.jar.
You can fool it by sym linking or copying gwt-dev.jar to the
appropriate name.


Breaking changes
* The way arguments are passed to the GWT testing infrastructure has
been revamped. There is now a consistent syntax to support arbitrary
runstyles, including user-written with no changes to GWT.  Though
this does not affect common launch configs, some of the less common
ones will need to be updated. For example, '-selenium FF3' has become
'-runStyle selenium:FF3'


As always, remember that GWT milestone builds like this are use-at-
your-own-risk and we don't recommend it for production use. Please
report any bugs you encounter to the GWT issue tracker (http://
code.google.com/p/google-web-toolkit/issues/list) after doing a quick
search to see if your issue has already been reported.

-- Amit Manjhi, on behalf of the Google Web Toolkit team
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---



[gwt-contrib] Re: setInnerText fails in HtmlUnit

2009-10-02 Thread Amit Manjhi
LGTM. Can you add a TODO to remove the annotation when we update to
HtmlUnit-2.7 or above?

On Fri, Oct 2, 2009 at 10:40 AM, jlaba...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 Description:
 After r6263 (run HtmlUnit FF3 using gecko1_8 instead of gecko), an
 HtmlUnit bug was uncovered where it does not clear out all elements when
 setInnerText() is called.

 Supposedly this is a known issue that was fixed in the branch.  This
 patch fixes it in trunk.

 Fix:
 Adding @DoNotRunWith(Platform.Htmlunit) to both tests.

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

 Affected files:
  user/test/com/google/gwt/dom/client/ElementTest.java
  user/test/com/google/gwt/user/client/ui/DOMTest.java


 Index: user/test/com/google/gwt/dom/client/ElementTest.java
 ===
 --- user/test/com/google/gwt/dom/client/ElementTest.java(revision
 6284)
 +++ user/test/com/google/gwt/dom/client/ElementTest.java(working
 copy)
 @@ -524,6 +524,7 @@
   /**
* innerText.
*/
 +  @DoNotRunWith(Platform.Htmlunit)
   public void testSetInnerText() {
 Document doc = Document.get();

 Index: user/test/com/google/gwt/user/client/ui/DOMTest.java
 ===
 --- user/test/com/google/gwt/user/client/ui/DOMTest.java(revision
 6284)
 +++ user/test/com/google/gwt/user/client/ui/DOMTest.java(working
 copy)
 @@ -293,6 +293,7 @@
* Tests that {...@link DOM#setInnerText(Element, String)} works
 consistently
* across browsers.
*/
 +  @DoNotRunWith(Platform.Htmlunit)
   public void testSetInnerText() {
 Element tableElem = DOM.createTable();





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



[gwt-contrib] Re: Fix GC handling in hosted mode

2009-09-25 Thread Amit Manjhi
LGTM.  Why use a TreeMap in RemoteObjectTable?

On Thu, Sep 24, 2009 at 4:26 PM, j...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 This corrects an issue that was there before and implements it on the
 client side of BrowserChannel, as needed by HtmlUnit in hosted mode.

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

 Affected files:
  dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java
  dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelClient.java
  dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java
  dev/oophm/src/com/google/gwt/dev/shell/JsValueOOPHM.java
  dev/oophm/src/com/google/gwt/dev/shell/ObjectsTable.java
  dev/oophm/src/com/google/gwt/dev/shell/OophmSessionHandler.java
  dev/oophm/src/com/google/gwt/dev/shell/RemoteObjectTable.java
  dev/oophm/src/com/google/gwt/dev/shell/ServerObjectsTable.java
  dev/oophm/test/com/google/gwt/dev/shell/BrowserChannelTest.java
  dev/oophm/test/com/google/gwt/dev/shell/RemoteObjectTableTest.java
  user/src/com/google/gwt/junit/RunStyle.java




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



[gwt-contrib] Re: Fix GC handling in hosted mode

2009-09-25 Thread Amit Manjhi
Yes.

On Fri, Sep 25, 2009 at 10:35 AM, John Tamplin j...@google.com wrote:

 On Fri, Sep 25, 2009 at 1:22 PM, Amit Manjhi amitman...@google.comwrote:

 LGTM.  Why use a TreeMap in RemoteObjectTable?


 I basically kept the same thing that Bob had there before, just moved it
 from ThreadLocals to an isolated synchronized class.  I don't see any reason
 it needs to be ordered, so I will change it to a HashMap.

 Ok to commit with that change?

 --
 John A. Tamplin
 Software Engineer (GWT), Google


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



[gwt-contrib] Re: Ant file changes for removing swt and mac-os specific stuff

2009-09-24 Thread Amit Manjhi
Nope, that can be inlined. Will commit with this change. Thanks!

On Thu, Sep 24, 2009 at 12:57 PM, Freeland Abbott fabb...@google.comwrote:

 Also, is there a reason we can't discard junit.headless from
 common.ant.xml, rather than hardcoding it true?


 On Thu, Sep 24, 2009 at 3:53 PM, j...@google.com wrote:

 LGTM

 Reitveld is again acting up -- I couldn't actually view any of the files
 but the first and last, but I could get a diff of the whole patch to
 look at.


 http://gwt-code-reviews.appspot.com/71804/diff/1/2
 File common.ant.xml (right):

 http://gwt-code-reviews.appspot.com/71804/diff/1/2#newcode185
 Line 185: jvmarg line=-Xmx2048m /
 I don't know that we want to increase this in the build file.

 http://gwt-code-reviews.appspot.com/71804/diff/1/7
 File user/build.xml (right):

 http://gwt-code-reviews.appspot.com/71804/diff/1/7#newcode145
 Line 145: gwt.junit test.args=${test.args}
 test.out=${junit.out}/${build.host.platform}-hosted-mode
 test.cases=test.hosted.tests 
 I assume this is temporary?


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




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



[gwt-contrib] Re: Ant file changes for removing swt and mac-os specific stuff

2009-09-24 Thread Amit Manjhi
The -batch module and bumping the heap changes are temporary. Actually,
I will leave them out of the commit.

On Thu, Sep 24, 2009 at 12:53 PM, j...@google.com wrote:

 LGTM

 Reitveld is again acting up -- I couldn't actually view any of the files
 but the first and last, but I could get a diff of the whole patch to
 look at.


 http://gwt-code-reviews.appspot.com/71804/diff/1/2
 File common.ant.xml (right):

 http://gwt-code-reviews.appspot.com/71804/diff/1/2#newcode185
 Line 185: jvmarg line=-Xmx2048m /
 I don't know that we want to increase this in the build file.

 http://gwt-code-reviews.appspot.com/71804/diff/1/7
 File user/build.xml (right):

 http://gwt-code-reviews.appspot.com/71804/diff/1/7#newcode145
 Line 145: gwt.junit test.args=${test.args}
 test.out=${junit.out}/${build.host.platform}-hosted-mode
 test.cases=test.hosted.tests 
 I assume this is temporary?


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


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



[gwt-contrib] Re: Add more HtmlUnit platforms/browsers testings

2009-09-09 Thread Amit Manjhi
On Wed, Sep 9, 2009 at 11:19 AM, Frank Lin f...@google.com wrote:

 Hi Amit,


 On Wed, Sep 9, 2009 at 9:25 AM, amitman...@google.com wrote:

 Hi Frank,

 Instead of having individual ant commands for each browser emulation, it
 would be better to test all the browsers in parallel using -htmlunit
 FF3, FF2, IE6, IE7.

 I would suggest more ant targets rather let ant optimize the parallelism
 since blaze might be able to shard them better. We might need Freeland's
 expertise on that.


When all browsers are specified, the parallelism is at the JVM level, not at
the ant level. A separate HtmlUnit thread is started to emulate each
browser.

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



[gwt-contrib] Re: Add more HtmlUnit platforms/browsers testings

2009-09-09 Thread Amit Manjhi
For now, it seems fine. It is the same issue when running multiple browsers
simultaneously during remoteweb or selenium tests.

On Wed, Sep 9, 2009 at 1:45 PM, Frank Lin f...@google.com wrote:

 Just one issue: is it okay to use test.out for all platforms if using
 '-htmlunit FF3, FF2, IE6, IE7'?
  test.out=${junit.out}/${build.host.platform}-htmlunit-web-mode

 On Wed, Sep 9, 2009 at 11:40 AM, Amit Manjhi amitman...@google.comwrote:



 On Wed, Sep 9, 2009 at 11:19 AM, Frank Lin f...@google.com wrote:

 Hi Amit,


 On Wed, Sep 9, 2009 at 9:25 AM, amitman...@google.com wrote:

 Hi Frank,

 Instead of having individual ant commands for each browser emulation, it
 would be better to test all the browsers in parallel using -htmlunit
 FF3, FF2, IE6, IE7.

 I would suggest more ant targets rather let ant optimize the parallelism
 since blaze might be able to shard them better. We might need Freeland's
 expertise on that.


 When all browsers are specified, the parallelism is at the JVM level, not
 at the ant level. A separate HtmlUnit thread is started to emulate each
 browser.




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



[gwt-contrib] Re: Call webClient.closeAllWindows() in htmlunit code

2009-08-20 Thread Amit Manjhi
Not while a test is running. Only when a test or a block of test ends does
the GWTRunner ping again.

On Thu, Aug 20, 2009 at 3:38 PM, Scott Blum sco...@google.com wrote:

 Does JUnit's GWTRunner keep pinging the server on a timer to see if more
 tests are wanted?

 On Thu, Aug 20, 2009 at 5:04 PM, kpro...@google.com wrote:


 In a discussion with Amit and Joel, we've determined that the tests that
 this will apply to do not use History tokens, so the repeated timeout
 won't apply.  We couldn't think of any other cases where there would be
 lingering JavaScript jobs. TODO - we should revisit this issue at some
 point and come up with something more stable.

 On 2009/08/20 20:27:25, kathrin wrote:
  Hi Amit,

  I think this is the right direction!  But I do think there's still a
 problem
  here, if I read the HtmlUnit documentation correctly.  It says:

  -
  waitForBackgroundJavaScriptStartingBefore

   This method blocks until all background JavaScript tasks scheduled
 to start
  executing before (now + delayMillis) have finished executing.
 Background
  JavaScript tasks are JavaScript tasks scheduled for execution via
  window.setTimeout, window.setInterval or asynchronous XMLHttpRequest.
  ...

   Returns:
   the number of background JavaScript jobs still executing or
 waiting to
  be executed when this method returns; will be 0 if there are no jobs
 left to
  execute
  -

  Now what's happening here is that you wait x ms for the jobs to
 finish, but then
  there will at least be 1 still running (because GWT apps have a
 setTimeout(200?)
  that keeps setting a setTimeout(200) to check whether the history has
 changed.
  So unless I'm missing something, every time this method returns, it'll
 return a
  value  0, which means you'll just loop around until the test times
 out. Am I
  missing something?

  http://gwt-code-reviews.appspot.com/62802/diff/1/2
  File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right):

  http://gwt-code-reviews.appspot.com/62802/diff/1/2#newcode101
  Line 101: + JAVASCRIPT_WAIT_TIME +  ms);
  Rephrase a bit?  Maybe say Waiting for (++count) javascript jobs for
 xxx ms



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

 



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



[gwt-contrib] Re: [google-web-toolkit] r5897 committed - Rolling back tools and trunk r5889 through 5892 due to ...

2009-08-06 Thread Amit Manjhi
This case happens to be in the exception category.

These were files that I had added a couple of days ago. And only my code
depended on it. I found a way to use already existing libraries from tools
instead of these libs. Since there was no other use for them, I deleted them
after consulting with Ray.
On Thu, Aug 6, 2009 at 10:33 AM, Scott Blum sco...@google.com wrote:

 Nothing should be deleted from tools/ ever.  The only exceptions would be
 something added by mistake that no version of GWT depends on.


 On Wed, Aug 5, 2009 at 11:58 PM, codesite-nore...@google.com wrote:


 Revision: 5897
 Author: rj...@google.com
 Date: Wed Aug  5 20:57:41 2009
 Log: Rolling back tools and trunk r5889 through 5892 due to
 build break


 http://code.google.com/p/google-web-toolkit/source/detail?r=5897

 Added:
  /tools/lib/xerces/serializer-2.7.1.jar
  /tools/lib/xerces/xercesImpl-2.8.1.jar
  /tools/lib/xerces/xml-apis-1.3.04.jar

 ===
 --- /dev/null
 +++ /tools/lib/xerces/serializer-2.7.1.jar  Wed Aug  5 20:57:41 2009
 Binary file, no diff available.
 ===
 --- /dev/null
 +++ /tools/lib/xerces/xercesImpl-2.8.1.jar  Wed Aug  5 20:57:41 2009
 File is too large to display a diff.
 ===
 --- /dev/null
 +++ /tools/lib/xerces/xml-apis-1.3.04.jar   Wed Aug  5 20:57:41 2009
 Binary file, no diff available.

 



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



[gwt-contrib] Re: Java changes for OOPHM wire protocol change

2009-08-06 Thread Amit Manjhi
Yeah, the enhanced-for comment for assigning to array elements is dumb. My
bad.
Please feel free to submit after making changes.

On Thu, Aug 6, 2009 at 12:46 PM, j...@google.com wrote:

 Thanks for the review.


 http://gwt-code-reviews.appspot.com/51835/diff/3001/3006
 File dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html
 (right):

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode23
 Line 23: $legacyHosted = true;
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Can we get rid of all code related to legacyHosted?


 Not yet, since people are still using it.

 When SWT is removed, this would be removed at the same time.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode205
 Line 205: }
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Can make this clearer using ASCII_EXCLAMATION = 33 and ASCII_TILDA =

 127. The

 comment is then redundant.


 Ok.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode238
 Line 238:
 loadIframe(
 http://code.google.com/p/google-web-toolkit/wiki/TroubleshootingOOPHM;);
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Does this wiki not exist yet? I could not find it.


 No.  I will create at least a template before committing.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3008
 File dev/core/src/com/google/gwt/dev/SwtHostedModeBase.java (right):

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3008#newcode80
 Line 80: String remoteEndpoint) throws UnableToCompleteException {
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 a more descriptive name than string? Missing javadoc on a public

 method. If

 this class will be removed in 2.0, I suppose you can leave it as such.



 The auto-refactor of the implementing class did this, and I have already
 fixed it locally.  In fact, it is url, sessionKey rather than
 sessionKey, string.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3003
 File dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java (right):

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode621
 Line 621: }
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Use enhanced-for loop?


 How would the enhanced for work here?  I am assigning to the array
 elements.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode643
 Line 643: for (int i = 0; i  n; ++i) {
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Use enhanced-for loop?


 Ok.

 http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode662
 Line 662: }
 On 2009/08/06 19:17:50, amitmanjhi wrote:

 Use enhanced-for loop?


 Again, doesn't work assigning to the array.


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


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



[gwt-contrib] Re: patch to enable web tests using HtmlUnit

2009-07-30 Thread Amit Manjhi
Thanks Scott. Commited the changes at r5844 with a fix the first thing and
 a TODO for the second thing.

On Thu, Jul 30, 2009 at 10:48 AM, sco...@google.com wrote:

 LGTM with nits.


 http://gwt-code-reviews.appspot.com/54809/diff/1096/1130
 File user/src/com/google/gwt/junit/JUnitShell.java (right):

 http://gwt-code-reviews.appspot.com/54809/diff/1096/1130#newcode220
 Line 220: + e.g. IE6,IE7,FF2,FF3...;
 Could the exact list be gotten from RunStyleHtmlUnit?  Then you don't
 need to update this message if the set of browsers changes.

 http://gwt-code-reviews.appspot.com/54809/diff/1096/1129
 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right):

 http://gwt-code-reviews.appspot.com/54809/diff/1096/1129#newcode149
 Line 149: threads.add(hut);
 Is there any way to, say, pause here waiting for the other thread to get
 to an okay state that would could call a successful launch?  Might be
 nice to detect  report errors early.


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


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



[gwt-contrib] Re: Add emma_ant.jar to tools directory

2009-07-23 Thread Amit Manjhi
emma.jar includes our modifications to emma. That is why redist was chosen
instead of lib.
The version is included in the name of zip file: emma-2.0.5312-src.zip When
the zip file is expanded, it just includes 'emma.jar' with no version
number. So, it made sense to make emma with our patch available under the
same name.

On Thu, Jul 23, 2009 at 11:53 AM, Scott Blum sco...@google.com wrote:

 Also, don't we need a version on emma_ant.jar?  There should have been one
 put on emma.jar, but it looks like an oversight.


 On Thu, Jul 23, 2009 at 2:51 PM, Scott Blum sco...@google.com wrote:

 Why does emma live in redist/ instead of /lib anyway?


 On Thu, Jul 23, 2009 at 2:47 PM, jlaba...@google.com wrote:


 Reviewers: bruce,

 Description:
 This patch adds the emma_ant jar to our tools directory so the GWT build
 file can use it to generate code coverage data.  We already include
 emma.jar.

 A subsequent patch to trunk will update the build files to add code
 coverage stats.

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

 Affected files:
   redist/emma/README.txt
   redist/emma/emma_ant.jar


 Index: redist/emma/README.txt
 ===
 --- redist/emma/README.txt  (revision 5777)
 +++ redist/emma/README.txt  (working copy)
 @@ -9,6 +9,9 @@

  emma.jar: the jar file produced by the patched emma.

 +emma_ant.jar: the jar file containing custom EMMA ant tasks that can be
 +downloaded from sourceforge.net.
 +
  com.mountainminds.eclemma.core_1.3.2.jar : eclemma jar containing the
 updated
  emma.jar

 Index: redist/emma/emma_ant.jar
 ===
 Cannot display: file marked as a binary type.
 svn:mime-type = application/octet-stream

 Property changes on: redist/emma/emma_ant.jar
 ___
 Name: svn:mime-type
+ application/octet-stream








 


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



[gwt-contrib] Re: apicheck should run on Windows

2009-07-11 Thread Amit Manjhi
Good catch. LGTM

On Fri, Jul 10, 2009 at 9:40 PM, fabb...@google.com wrote:

 Reviewers: amitmanjhi,

 Description:
 the old form got confused by a refJar path of

 'C:\GWT\tools\api-checker\reference\gwt-dev-modified.jar:C:\GWT\tools\api-checker\reference\gwt-user-modified.jar'
 because splitting on colons was wrong.  Worse, ant thought that it had
 succeeded because an arg-parsing error was caught, help printed, and
 ultimately 0 exited.

 The new form splits on the path.separator system propery instead, uses
 ant's arg path=.../ to input either a : or ; separated value depending
 on OS, and catches arg parse failures as non-zero return.

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

 Affected files:
  build.xml

  
 tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java




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



[gwt-contrib] Re: support for internationalized svninfo

2009-07-08 Thread Amit Manjhi
I also tried this patch. Build works great. But, 'ant test
-dgwt.svnrev=...@yyy' fails because com.google.gwt.ant.taskdefs.SvnInfoTest
fails. If the ant property is defined, should the SvnInfoTest be disabled as
well?

Regards,
Amit

On Wed, Jul 8, 2009 at 3:22 AM, chassa...@gmail.com wrote:


 hi,

 Today I updated my gwt with the trunk (rev 5687). I removed my changes
 (uncomment lines). I applied the patch on the file :
 ./build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java
 Now it works. I didn't need to create a local.ant.properties file.

 Thanks !

 Best regards,
 Seb

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

 


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



[gwt-contrib] Re: Specify the path for translatable code explicitly in new apps created by webAppCreator

2009-06-12 Thread Amit Manjhi
Thanks Scott and Ray. Commited at r.

On Fri, Jun 12, 2009 at 4:11 PM, Ray Ryan rj...@google.com wrote:

 LGTM++


 On Fri, Jun 12, 2009 at 4:08 PM, Scott Blum sco...@google.com wrote:

 LGTM


 On Fri, Jun 12, 2009 at 6:49 PM, amitman...@google.com wrote:

 Reviewers: Ray Ryan, scottb,

 Description:
 It will be less confusing for new users. Anyone see any downsides?

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

 Affected files:
  user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc


 Index: user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc
 ===
 --- user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc(revision
 5552)
 +++ user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc(working
 copy)
 @@ -14,4 +14,8 @@

   !-- Specify the app entry point class. --
   entry-point class='@clientpacka...@moduleshortname'/
 +
 +  !-- Specify the paths for translatable code--
 +  source path='client'/
 +
  /module






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



[gwt-contrib] Re: Speed up compilation by rewriting for loops to avoid iterators

2009-06-04 Thread Amit Manjhi
I completely agree with Scott here. It's hard to believe that there will be
a consistent 3% improvement just by using an explicit index. The change is
very much compiler/JVM dependent. And even if there were the case currently,
the next compiler/JVM could very easily fix this.

Since I was surprised by the result, I wrote up a simple micro-benchmark
that repeats this test with Integer lists of varying sizes. I ran it with
openjdk and did not see *any*  performance difference. I have attached the
quick-and-dirty code and included the sample results at the end.

Amit

=
Here are sample results: java -cp . -Xmx1500M com.test.Test

iteration with 1000 elements
long way: 1, short way: 1
long way: 1, short way: 1
long way: 2, short way: 3
long way: 1, short way: 3
long way: 2, short way: 5
long way: 9, short way: 7
long way: 1, short way: 2
long way: 10, short way: 2
long way: 0, short way: 2
long way: 0, short way: 1


iteration with 100 elements
long way: 11, short way: 12
long way: 14, short way: 11
long way: 20, short way: 20
long way: 20, short way: 22
long way: 27, short way: 27
long way: 28, short way: 30
long way: 38, short way: 42
long way: 37, short way: 42
long way: 45, short way: 45
long way: 46, short way: 49


iteration with 1000 elements
long way: 124, short way: 124
long way: 125, short way: 125
long way: 280, short way: 262
long way: 282, short way: 262
long way: 349, short way: 345
long way: 348, short way: 344
long way: 529, short way: 499
long way: 566, short way: 663
long way: 530, short way: 508
long way: 530, short way: 508



On Thu, Jun 4, 2009 at 1:58 PM, Aaron Steele eightyste...@gmail.com wrote:


 Do'h! Yeah, using the name 'ints' probably wasn't a good choice here.
 Looks like I should re-read Item 56: Adhere to generally accepted
 naming conventions. :)

 On Thu, Jun 4, 2009 at 1:09 PM, Alex Rudnick a...@google.com wrote:
 
  Yeesh, pardon. That's an ArrayList called ints of Integers, not
  containing ints. I retract that statement!
 
  On Thu, Jun 4, 2009 at 4:04 PM, Alex Rudnick a...@google.com wrote:
  Sounds like boxing/unboxing overhead, in that case!
 
  What if you tried that with an array of native ints?
 
  On Thu, Jun 4, 2009 at 3:47 PM, Aaron Steele eightyste...@gmail.com
 wrote:
 
  So item 46 in Effective Java says that there shouldn't be a
  performance penalty using the nice for loops. But the following test
  in Eclipse on my machine (MacBook Pro, Intel Core Duo, 2.16 GHz) shows
  a performance penalty.
 
  Given an ArrayList called ints with 1 million Integers, this takes 31
  milliseconds:
  for (int i = 0, size = ints.size(); i  size; i++)
   ints.get(i).intValue();
 
  And this takes 76 milliseconds:
  for (Integer i : ints)
   i.intValue();
 
  What am I missing? Probably just some super naive testing on my part.
 :)
 
  --
  Alex Rudnick
  swe, gwt, atl
 
  
 



 --

 Sent from Piedmont, CA, United States

 


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

/*
 * Copyright 2008 Google Inc.
 * 
 * Licensed under the Apache License, Version 2.0 (the License); you may not
 * use this file except in compliance with the License. You may obtain a copy of
 * the License at
 * 
 * http://www.apache.org/licenses/LICENSE-2.0
 * 
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an AS IS BASIS, WITHOUT
 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 * License for the specific language governing permissions and limitations under
 * the License.
 */
package com.test;

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

/**
 * An implementation for Test.
 */
public class Test {
  static final int ITERATIONS = 5;

  public static void main(String args[]) {
ListInteger integerList = new ArrayListInteger();

for (int maxSize : new int[] {1000, 1000 * 1000, 1000 * 1000 * 10}) {
  long start = 0, shortWay = 0, longWay = 0;
  System.out.println(\n\niteration with  + maxSize +  elements);
  for (int i = 0; i  ITERATIONS; i++) {
constructList(integerList, maxSize);
start = System.currentTimeMillis();
findMaxLongWay(integerList);
longWay = System.currentTimeMillis() - start;
findMaxShortWay(integerList);
shortWay = System.currentTimeMillis() - start - longWay;
System.out.println(long way:  + longWay + , short way:  + shortWay);

// repeat in reverse order to get rid of memory/caching effects
start = System.currentTimeMillis();
findMaxShortWay(integerList);
shortWay = System.currentTimeMillis() - start;
findMaxLongWay(integerList);
longWay = System.currentTimeMillis() - start - shortWay;
System.out.println(long way:  + longWay + , short way:  + shortWay);
 

[gwt-contrib] patch to fix issue 2979

2009-05-13 Thread Amit Manjhi
Hi John/Scott,

Please review this simple patch that fixes issue 2979.

Regards,
Amit

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

Index: dev/windows/src/org/eclipse/swt/browser/WebSite.java
===
--- dev/windows/src/org/eclipse/swt/browser/WebSite.java	(revision 5360)
+++ dev/windows/src/org/eclipse/swt/browser/WebSite.java	(working copy)
@@ -38,8 +38,9 @@
  */
 public boolean startGears() {
   // Get the classID of the Gears BHO.
+  GUID appClsid = null;
   try {
-GUID appClsid = getClassID(gears.BHO);
+appClsid = getClassID(gears.BHO);
 if (appClsid == null) {
   return false;
 }


[gwt-contrib] Re: patch to fix issue 2979

2009-05-13 Thread Amit Manjhi
Thanks! Seems like it never worked.

Commited as r5364.

On Wed, May 13, 2009 at 1:03 PM, Scott Blum sco...@google.com wrote:

 LGTM, but how could this have ever worked?


 On Wed, May 13, 2009 at 3:32 PM, amitman...@google.com wrote:

 Reviewers: jat, scottb,

 Description:

 Hi John/Scott,

 Please review this simple patch that fixes issue 2979.

 Regards,
 Amit

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

 Affected files:
  dev/windows/src/org/eclipse/swt/browser/WebSite.java


 Index: dev/windows/src/org/eclipse/swt/browser/WebSite.java
 ===
 --- dev/windows/src/org/eclipse/swt/browser/WebSite.java(revision
 5360)
 +++ dev/windows/src/org/eclipse/swt/browser/WebSite.java(working
 copy)
 @@ -38,8 +38,9 @@
  */
  public boolean startGears() {
   // Get the classID of the Gears BHO.
 +  GUID appClsid = null;
   try {
 -GUID appClsid = getClassID(gears.BHO);
 +appClsid = getClassID(gears.BHO);
 if (appClsid == null) {
   return false;
 }





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



[gwt-contrib] Re: Fix for issue 3057

2009-05-13 Thread Amit Manjhi
Thanks! Commited as r5366.

On Wed, May 13, 2009 at 2:24 PM, Scott Blum sco...@google.com wrote:

 LGTM


 On Wed, May 13, 2009 at 5:00 PM, amitman...@google.com wrote:

 Reviewers: jat, scottb,

 Description:
 Hi John/Scott,

 Please review this patch that fixes issue 3057. I also added testcases
 and verified that the testcases pass in both web and hosted mode.

 Thanks,
 Amit

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

 Affected files:
  user/super/com/google/gwt/emul/java/util/AbstractList.java

  user/test/com/google/gwt/emultest/java/util/AbstractSequentialListTest.java





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



[gwt-contrib] Re: Fix for issue 3422

2009-05-13 Thread Amit Manjhi
The last import in TreeMapStringStringTest.java is unnecessary. Will remove
it when I commit the patch.

On Wed, May 13, 2009 at 4:15 PM, amitman...@google.com wrote:

 Reviewers: jat, scottb,

 Description:
 Simple fix and accompanying testcase for toString() method in
 TreeMap.Entry

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

 Affected files:
  user/super/com/google/gwt/emul/java/util/TreeMap.java
  user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java


 Index: user/super/com/google/gwt/emul/java/util/TreeMap.java
 --- user/super/com/google/gwt/emul/java/util/TreeMap.java   (revision
 3877)
 +++ user/super/com/google/gwt/emul/java/util/TreeMap.java   (working
 copy)
 @@ -234,7 +234,8 @@ public class TreeMapK, V extends AbstractMapK, V
 implements

 @Override
 public String toString() {
 -  return (isRed ? R:  : B: ) + key + = + value;
 +  // for compatibility with the real Jre: issue 3422
 +  return key + = + value;
 }
   }

 Index:
 user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java
 ---
 user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java
(revision 3589)
 +++
 user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java
(working copy)
 @@ -16,9 +16,12 @@
  package com.google.gwt.emultest.java.util;

  import java.util.Iterator;
 +import java.util.Map;
  import java.util.NoSuchElementException;
  import java.util.Set;
  import java.util.SortedMap;
 +import java.util.TreeMap;
 +import java.util.Map.Entry;

  /**
  * Tests codeTreeMap/code with Strings and the natural comparator.
 @@ -118,6 +121,15 @@ public class TreeMapStringStringTest extends
 TreeMapTestString, String {
 assertEquals(lastKey, dd, subMap.lastKey());
   }

 +  // checks for compatibility with real Jre's Entry.toString(): issue 3422
 +  public void testTreeMapEntryToString() {
 +MapString, String treeMap = new TreeMapString, String();
 +treeMap.put(bar, barValue);
 +
 +assertEquals(bar=barValue,
 +treeMap.entrySet().iterator().next().toString());
 +  }
 +
   @Override
   protected Object getConflictingKey() {
 return new Integer(1);





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



  1   2   >