[gwt-contrib] Re: one-word review for trivial patch in About.java

[Freeland Abbott]

Submitted at r3646.

On Wed, Sep 10, 2008 at 11:14 PM, Freeland Abbott wrote:

> I actually looked at the TODO to figure out a way to consolidate most of
> that stuff, which would achieve much the same effect.  But I decided I
> didn't want to expand this patch...
>
> (A subdir of ${project.build} actually hits the same problem, since we copy
> ${project.build}... I think you meant a sibling of it?)
>
>
>
> On Wed, Sep 10, 2008 at 7:10 PM, Scott Blum <[EMAIL PROTECTED]> wrote:
>
>> This LGTM.  The one thing I might tweak involves the changes to
>> /distro-source//build.xml.  Basically, the change you made
>> highlights the brittleness of slurping in all of ${project.build}, since we
>> might continue to introduce tools like revision filtering that add new stuff
>> to ${project.build} automatically.
>> Perhaps we should tweak the distro source build to put the copied files
>> into a subdirectory, like ${project.build}/filtered.
>>
>>
>> On Wed, Sep 10, 2008 at 2:14 PM, Freeland Abbott <
>> [EMAIL PROTECTED]> wrote:
>>
>>> On Mon, Sep 8, 2008 at 10:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:
>>>
 Let me rephrase the question: couldn't we have two sentinel files
 instead of four?  Right now you have two sentinel files for dev/core and 
 two
 for distro-source; each could get by with only one sentinel file whose name
 composited gwt.version with gwt.svnrev.filename.

>>>
>>> Ah.  Yes, we could have two sentinels rather than four, sorry---I
>>> misunderstood which pair(s) you wanted to fold.
>>>
>>>
 Detailed review:

 At a high level, I'm not sure why you went the route of making things so
 general.  Why does gwt.svninfo need to take parameters to say what props to
 read the values into?  Why not simply always read them into {gwt.svnrev} 
 and
 ${gwt.svnrev.filename}?  For that matter, why does gwt.revfilter need to
 take parameters for those values?  I think both of these should use
 hardcoded property names; the other advantage to this is the ability to
 override these values on the command line and know you've gotten them
 globally.

>>>
>>> I like macros not to have side effects.  Not strongly enough to contest
>>> it, so the new patch has hardcoded output properties, but that was the
>>> motivation.  (It also would avoid any namespace issues when, inevitably,
>>> this gets cut-and-pasted into the "common".ant.xml of incubator,
>>> gwt-google-apis, etc... though I know those examples have no namespace
>>> issues today anyway.)
>>>
>>>
 Ideally, those properties could be computed at the top level without
 actually having to remember to call the macro to get them defined, but I'm
 not sure that that's worth the effort.

>>>
>>> While setting the value should be pretty "fast," if you're working just
>>> in user you may rather not have it set since it won't ever be used.  (We
>>> only have two use instances; I'd swing the other way with a third one.)  So,
>>> yeah, I passed on this one: you need to call  in any
>>> subproject that cares.  That does, however, establish common naming for the
>>> sentinels and creates the sentinel directory, which cleans up some stuff.
>>>
>>> The distro-source used to skip sentinels entirely, so there's also some
>>> added exclusion there now that they're creating 'em.
>>>
>>>
>>>
 /dev/common.ant.xml
 - Why do you need to exclude About.properties from both source
 directories, since it will only exist in dev/core?

>>>
>>> I'd expected common to be common-inclusive-of-core, not just
>>> common-to-plaforms-only.  ("Expected" is maybe too strong; "programmed to
>>> allow for" is perhaps better---but if core included common, then inside core
>>> "src" would be an alias for ${gwt.core.root}/src and would need the
>>> exclusion.)  Today, not needed, so out it goes.
>>>
>>>
 /distro-source/common.ant.xml
 - Again, 24-25 & 40-41 can be merged to a single sentinel file.
 - 21: What's with the echo here?

>>>
>>> Yup, and debuggage now gone.
>>>
>>
>>
>> >>
>>
>

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

[gwt-contrib] [google-web-toolkit commit] r3646 - in trunk: . dev dev/core dev/core/src/com/google/gwt/dev distro-source distro-source/linu...

[codesite-noreply]

Author: [EMAIL PROTECTED]
Date: Wed Sep 10 20:19:19 2008
New Revision: 3646

Modified:
trunk/common.ant.xml
trunk/dev/common.ant.xml
trunk/dev/core/build.xml
trunk/dev/core/src/com/google/gwt/dev/About.java
trunk/distro-source/common.ant.xml
trunk/distro-source/linux/build.xml
trunk/distro-source/mac/build.xml
trunk/distro-source/windows/build.xml

Log:
Change instance initializer in About.java to static.  Most of the work,  
though, was on the Ant files such that once the filtered copies have  
already been generated, they will re-generate IFF the gwt OR svn versions  
have changed (i.e. after an svn up).

Review by: scottb

Modified: trunk/common.ant.xml
==
--- trunk/common.ant.xml(original)
+++ trunk/common.ant.xmlWed Sep 10 20:19:19 2008
@@ -199,10 +199,7 @@
  


-  
-
-
+  
  

@@ -227,7 +224,31 @@
  casesensitive="false" />

+  
+  
+  
+  
+  
+
+  
+
+  
+
+
+
+

+  


  
@@ -236,7 +257,6 @@

  

-  
  



Modified: trunk/dev/common.ant.xml
==
--- trunk/dev/common.ant.xml(original)
+++ trunk/dev/common.ant.xmlWed Sep 10 20:19:19 2008
@@ -19,8 +19,11 @@

  
  
-  
-  
+  
+  
+
+
+  




Modified: trunk/dev/core/build.xml
==
--- trunk/dev/core/build.xml(original)
+++ trunk/dev/core/build.xmlWed Sep 10 20:19:19 2008
@@ -57,12 +57,12 @@



-  
-
-
+  
+

  
-
+



@@ -76,22 +76,8 @@
  
  

-
-
-
-  
-
-
-  
-  
-
-  
-
-
-
  
  
-
  

  
@@ -102,8 +88,21 @@

  
  
-  
-
+  
+
+
+
+
+
+  
+
+
+  
+  
+
+  
+
+




Modified: trunk/dev/core/src/com/google/gwt/dev/About.java
==
--- trunk/dev/core/src/com/google/gwt/dev/About.java(original)
+++ trunk/dev/core/src/com/google/gwt/dev/About.javaWed Sep 10 20:19:19  
2008
@@ -32,13 +32,10 @@

public static String GWT_VERSION;

-  {
-Class myClass = this.getClass();
-String propsPath =  
myClass.getName().replace('.', '/').concat(".properties");
+  static {
  Properties props = new Properties();
  try {
-  InputStream instream = myClass.getClassLoader().getResourceAsStream(
-  propsPath);
+  InputStream instream =  
About.class.getResourceAsStream("About.properties");
props.load(instream);
  } catch (IOException iox) {
// okay... we use default values, then.

Modified: trunk/distro-source/common.ant.xml
==
--- trunk/distro-source/common.ant.xml  (original)
+++ trunk/distro-source/common.ant.xml  Wed Sep 10 20:19:19 2008
@@ -17,9 +17,24 @@



+
+
+  
+
+
+  
+  
+
+  
+
+
+  
+
+  
  

  
+




Modified: trunk/distro-source/linux/build.xml
==
--- trunk/distro-source/linux/build.xml (original)
+++ trunk/distro-source/linux/build.xml Wed Sep 10 20:19:19 2008
@@ -25,6 +25,7 @@


  
+


  

Modified: trunk/distro-source/mac/build.xml
==
--- trunk/distro-source/mac/build.xml   (original)
+++ trunk/distro-source/mac/build.xml   Wed Sep 10 20:19:19 2008
@@ -27,6 +27,7 @@


  
+


  

Modified: trunk/distro-source/windows/build.xml
==
--- trunk/distro-source/windows/build.xml   (original)
+++ trunk/distro-source/windows/build.xml   Wed Sep 10 20:19:19 2008
@@ -20,7 +20,9 @@



-  
+  
+
+  




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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[fred]

I find all three cases (add, remove, set legal values) useful. For
gwt-log I'd like to offer developers the redefine capability. In other
contexts I can see add/remove being useful as well.

Fred

On 9/10/08, John Tamplin <[EMAIL PROTECTED]> wrote:
> On Wed, Sep 10, 2008 at 7:22 PM, BobV <[EMAIL PROTECTED]> wrote:
>
>> I agree with Bruce's design.  The ability to widen an
>> initially-restricted set is compelling.
>>
>
> I am fine with that, but I do not think that set-property should be
> overloaded to both change the set of legal property values and to select the
> active property value depending on how many values are provided, nor do I
> think the current behavior of being able to set the active property value
> without changing the set of legal property values should be discarded.  To
> me, that means a new directive is required.
>
> It would also be nice if I could simply remove from the existing set without
> having to replace it, which leads to a maintenance landmine if in fact I
> just wanted to exclude one property.  For consistency, the same method could
> be used to add new values (like extend-property).
>
> So, for example, it could be something like this:
>
> To remove one value from the set:
> 
>
> To add a value while removing another:
> 
>
> To replace the set of values:
> 
>
> --
> John A. Tamplin
> Software Engineer (GWT), Google
>
> >
>


-- 
Fred Sauer
[EMAIL PROTECTED]

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

[gwt-contrib] [google-web-toolkit commit] r3645 - trunk/user/super/com/google/gwt/emul/java/lang

[codesite-noreply]

Author: [EMAIL PROTECTED]
Date: Wed Sep 10 17:52:51 2008
New Revision: 3645

Modified:
trunk/user/super/com/google/gwt/emul/java/lang/String.java

Log:
Peephole remove duplicate assignment.

Modified: trunk/user/super/com/google/gwt/emul/java/lang/String.java
==
--- trunk/user/super/com/google/gwt/emul/java/lang/String.java  (original)
+++ trunk/user/super/com/google/gwt/emul/java/lang/String.java  Wed Sep 10  
17:52:51 2008
@@ -678,8 +678,7 @@
}
  }
  var jr = @java.lang.String::__createArray(I)(out.length);
-var i = 0;
-for(i = 0; i < out.length; ++i) {
+for(var i = 0; i < out.length; ++i) {
jr[i] = out[i];
  }
  return jr;

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

[gwt-contrib] core review: two optimizations on array creation

[Scott Blum]

Hi Lex or Bob,
Could you please review these tweaks to array instantiation?

1) For reference-type arrays, we no longer need to initialize to 'null'
explicitly since 'undefined' works just as well.

2) Instead of caching a protoTypeArray and using for-in to copy expandos,
cache the names and values in a pair of arrays.  Array iteration is
generally faster than for-in.  This also uses the
"static-initializer-inner-class" pattern to avoid lazy checks and also
factor the code out.

Thanks,
Scott

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



array-opts-r3644.patch
Description: Binary data

[gwt-contrib] Re: RR : Allow to redefine an existing property

[John Tamplin]

On Wed, Sep 10, 2008 at 7:37 PM, BobV <[EMAIL PROTECTED]> wrote:

> They're the same thing.  The previous  behavior is simply
> limiting the number of active choices to exactly one.
>

As far as I can tell,  calls property.setActiveValue, which
simply updates the activeValue field.  This does not impact
property.getKnownValues(), which ulimiately comes from  via
property.addKnownValue().

>From ModuleSpacePropertyOracle, getPropertyValueSet returns getKnownValues,
while getPropertyValue (via computePropertyValue) uses getActiveValue.

So, it appears to me, the way it works now is that
PropertyOracle.getPropertyValueSet will return the set of values from
define-property, while PropertyOracle.getPropertyValue will return the
result from set-property, or if not set, whatever the current permutation is
using.

What am I missing?

As mentioned before, LocaleInfoGenerator uses getPropertyValueSet to
determine the list of locales which information needs to be provided for,
and that is independent of the locale currently being compiled (which is
used elsewhere, but not the relevant part of this example).  If set-property
with multiple values were used to replace the set of locales available
rather than simply to restrict the permutations to compile, the generated
code will in fact be different than if you compiled all the permutations and
only kept that one.  I think that is counter-intuitive at least.

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

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

On Wed, Sep 10, 2008 at 2:26 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:

>
>
> On Wed, Sep 10, 2008 at 2:08 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> I think I understand your concern - is it that you don't want to see the
>> issue swept under the rug?
>>
>
> Yes, that's definitely my concern. And we've lost tests in the past that I
> really wish had remained online. They also have a bad compounding effect
> where the longer it stays offline the harder it is to bring it back because
> someone checks in something that is actually broken and they assume that
> since the change passes all tests that everything is good. As a result, we
> have a really bad track record of bringing back tests.
>
>
>>
>> I don't know enough right now to fix the test personally, but I do get
>> asked about build failures quite often - at least I feel compelled to look
>> into them to make sure the test infrastructure is working.
>>
>
> And I definitely don't want you having to fix things just because you're
> doing the good deed of investigating breakages. I would rather see you
> issued a build sheriff badge where you can say: "Build is breaking too much.
> The problem seems to be here, can you (person who last edited the test or
> generally known to own it) please look into the most appropriate fix."
>

You can keep the badge, I want spurs.  Spurs that jingle...


>
>
>>
>> We had discussions about this particular test failure a few days ago and
>> the consensus was that we should comment out the test for the time being and
>> bump up the priority on fixing the underlying problem.  I updated issue 864
>> before committing the patch, which Joel assigned to John Labanca 2 days ago.
>>
>
>  Sounds like you guys have a proper plan to get it back online.
>
> Thanks,
> /kel
>
>
>>
>> On Wed, Sep 10, 2008 at 1:49 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:
>>
>>>
>>> On Wed, Sep 10, 2008 at 1:38 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
 The failure that's occurring is very similar to the problem described in
 #864.  The assumptions of the test or the code we are testing is wrong.  I
 don't feel comfortable adding a lot of conditional code in the test when we
 already know it is broken.

>>>
>>> So why not fix the test if it is operating on faulty assumptions?
>>> Disabling tests really should be a last resort and I'm just curious why this
>>> one is in last resort state. I feel like if it were in a state where it
>>> should be disabled, putting in conditional code shouldn't be that
>>> uncomfortable. Also, before disabling it you should also have a plan for
>>> getting it back online as well and tie that to a issue tracker item.
>>>
>>> /kel
>>>
>>>

 On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]>wrote:

> Have you considered disabling those tests only for the platforms where
> they are flaky?
> /kel
>
> On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>
>> If we already comment out some methods, then LGTM.  Joel doesn't think
>> it matters, and I just wanted to be consistent not realizing that we've
>> already used both comments and "disabled" methods.
>>
>> Thanks,
>> John LaBanca
>> [EMAIL PROTECTED]
>>
>>
>> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]>wrote:
>>
>>> I don't mind doing that.  There are several other methods in
>>> ImageTest commented out in the same way I did it.  Want me to do the 
>>> same
>>> for those?
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>
 I think we usually just rename the test to "disabledTestXXX" so
 JUnit doesn't run it.  Do you mind doing it that way instead, but 
 leaving
 the comment?  Otherwise, the next time somebody auto-formats, the 
 method
 will be formatted like a comment.

 Thanks,
 John LaBanca
 [EMAIL PROTECTED]



 On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]>wrote:

> Hello John,
>
> I would like for you to review this patch.
>
> The testChangeClippedImageToUnclipped() method fails intermittently
> in our continuous build system on Linux hosted mode.  This could be 
> related
> to issues 863 & 864
>
> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>
> I've commented out the test, as there are many other tests in this
> TestCase that are commented out due to the same issues.
>
> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>


>>>
>>>

[gwt-contrib] Re: RR : Allow to redefine an existing property

[BobV]

> I am fine with that, but I do not think that set-property should be
> overloaded to both change the set of legal property values and to select the
> active property value depending on how many values are provided, nor do I
> think the current behavior of being able to set the active property value
> without changing the set of legal property values should be discarded.  To
> me, that means a new directive is required.

They're the same thing.  The previous  behavior is simply
limiting the number of active choices to exactly one.

-- 
Bob Vawter
Google Web Toolkit Team

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[John Tamplin]

On Wed, Sep 10, 2008 at 7:22 PM, BobV <[EMAIL PROTECTED]> wrote:

> I agree with Bruce's design.  The ability to widen an
> initially-restricted set is compelling.
>

I am fine with that, but I do not think that set-property should be
overloaded to both change the set of legal property values and to select the
active property value depending on how many values are provided, nor do I
think the current behavior of being able to set the active property value
without changing the set of legal property values should be discarded.  To
me, that means a new directive is required.

It would also be nice if I could simply remove from the existing set without
having to replace it, which leads to a maintenance landmine if in fact I
just wanted to exclude one property.  For consistency, the same method could
be used to add new values (like extend-property).

So, for example, it could be something like this:

To remove one value from the set:


To add a value while removing another:


To replace the set of values:


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

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[BobV]

I agree with Bruce's design.  The ability to widen an
initially-restricted set is compelling.

New thread that introduces non-deferred-binding properties:
  
http://groups.google.com/group/Google-Web-Toolkit-Contributors/t/fa19b93ae079af12

-- 
Bob Vawter
Google Web Toolkit Team

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Scott Blum]

Ok, Fred wins.  I concede to Bruce.

On Wed, Sep 10, 2008 at 3:39 PM, <[EMAIL PROTECTED]> wrote:

>
> my use case is for gwt-log
> The library supports 7 compile time values. I'd like to ship with one
> level so compile time does mulitply by some number >1. Many use
> involve users compiling with 2 or 3 levels, which may not include my
> library default level. I now have seven gwt XML files to accomodate
> the use cases.
>
> Allowing the developer to redefine the allowed valued would be very useful.
>
> Fred
> On 9/10/08, BobV <[EMAIL PROTECTED]> wrote:
> >
> >> - A redefinition of a deferred binding property does not invalidate a
> >> previous "set-property" declaration.  If the previous "set-property"
> >> declaration is no longer a valid value in the redefined property, it is
> a
> >> static error.
> >
> > This would be problematic if you wanted to redefine the property to a
> > new set of values that is disjoint from the original set.
> >
> > --
> > Bob Vawter
> > Google Web Toolkit Team
> >
> > >
> >
>
>
> --
> Fred Sauer
> [EMAIL PROTECTED]
>
> >
>

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

[gwt-contrib] Re: one-word review for trivial patch in About.java

[Scott Blum]

This LGTM.  The one thing I might tweak involves the changes to
/distro-source//build.xml.  Basically, the change you made
highlights the brittleness of slurping in all of ${project.build}, since we
might continue to introduce tools like revision filtering that add new stuff
to ${project.build} automatically.
Perhaps we should tweak the distro source build to put the copied files into
a subdirectory, like ${project.build}/filtered.

On Wed, Sep 10, 2008 at 2:14 PM, Freeland Abbott <[EMAIL PROTECTED]
> wrote:

> On Mon, Sep 8, 2008 at 10:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:
>
>> Let me rephrase the question: couldn't we have two sentinel files instead
>> of four?  Right now you have two sentinel files for dev/core and two for
>> distro-source; each could get by with only one sentinel file whose name
>> composited gwt.version with gwt.svnrev.filename.
>>
>
> Ah.  Yes, we could have two sentinels rather than four, sorry---I
> misunderstood which pair(s) you wanted to fold.
>
>
>> Detailed review:
>>
>> At a high level, I'm not sure why you went the route of making things so
>> general.  Why does gwt.svninfo need to take parameters to say what props to
>> read the values into?  Why not simply always read them into {gwt.svnrev} and
>> ${gwt.svnrev.filename}?  For that matter, why does gwt.revfilter need to
>> take parameters for those values?  I think both of these should use
>> hardcoded property names; the other advantage to this is the ability to
>> override these values on the command line and know you've gotten them
>> globally.
>>
>
> I like macros not to have side effects.  Not strongly enough to contest it,
> so the new patch has hardcoded output properties, but that was the
> motivation.  (It also would avoid any namespace issues when, inevitably,
> this gets cut-and-pasted into the "common".ant.xml of incubator,
> gwt-google-apis, etc... though I know those examples have no namespace
> issues today anyway.)
>
>
>> Ideally, those properties could be computed at the top level without
>> actually having to remember to call the macro to get them defined, but I'm
>> not sure that that's worth the effort.
>>
>
> While setting the value should be pretty "fast," if you're working just in
> user you may rather not have it set since it won't ever be used.  (We only
> have two use instances; I'd swing the other way with a third one.)  So,
> yeah, I passed on this one: you need to call  in any
> subproject that cares.  That does, however, establish common naming for the
> sentinels and creates the sentinel directory, which cleans up some stuff.
>
> The distro-source used to skip sentinels entirely, so there's also some
> added exclusion there now that they're creating 'em.
>
>
>
>> /dev/common.ant.xml
>> - Why do you need to exclude About.properties from both source
>> directories, since it will only exist in dev/core?
>>
>
> I'd expected common to be common-inclusive-of-core, not just
> common-to-plaforms-only.  ("Expected" is maybe too strong; "programmed to
> allow for" is perhaps better---but if core included common, then inside core
> "src" would be an alias for ${gwt.core.root}/src and would need the
> exclusion.)  Today, not needed, so out it goes.
>
>
>> /distro-source/common.ant.xml
>> - Again, 24-25 & 40-41 can be merged to a single sentinel file.
>> - 21: What's with the echo here?
>>
>
> Yup, and debuggage now gone.
>

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

[gwt-contrib] Re: RR:JS collections

[John Tamplin]

On Wed, Sep 10, 2008 at 6:29 PM, Emily Crutcher <[EMAIL PROTECTED]> wrote:

> The benchmarks are somewhat interesting, as it turns out that memory
> allocation from one can serious screw up the others. Therefore here I've
> included each variant as separate benchmarks posted on the JsMap 
> wikipage.
>

I haven't looked at the code yet, but what about non-IE benchmarks?  Also,
it looks like these results show linear time, and don't specify exactly what
is being benchmarked -- unless you are including iteration in there, I would
expect any hashmap benchmarks to be sub-linear.

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

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

[gwt-contrib] Comment on I18nAnnotations in google-web-toolkit

[codesite-noreply]

Comment by [EMAIL PROTECTED]:

Do you have an authoritive reference (preferably in English, but any is  
better than none) for this?  The Unicode CLDR data, which recently went  
through a large review with native speakers to get correct plural rules,  
includes the different plural rule for pt_BR than pt -- see  
http://www.unicode.org/cldr/data/charts/supplemental/language_plural_rules.html 
 
(note that they made Brazilian Portuguese the default and defined pt_PT as  
the special-case given the relative population sizes).


For more information:
http://code.google.com/p/google-web-toolkit/wiki/I18nAnnotations

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

[gwt-contrib] Comment on I18nAnnotations in google-web-toolkit

[codesite-noreply]

Comment by qatestaccount20:

I've seen it spread throughout the Internet that Brazilian Portuguese has  
the same noun form for both "zero" and "one". This is not standard nor  
spoken Brazilian Portuguese. Brazilian and Iberian Portuguese share the  
same plural form rules.

This unexpected rule is used by Mozilla for localization, but it's only  
there because it has never been reviewed by any Brazilian  
Portuguese-speaker. The rule is also stated by the English Wikipedia.


For more information:
http://code.google.com/p/google-web-toolkit/wiki/I18nAnnotations

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

[gwt-contrib] Comment on I18nAnnotations in google-web-toolkit

[codesite-noreply]

Comment by qatestaccount20:

I've seen it spread throughout the Internet that Brazilian Portuguese has  
the same noun form for both "zero" and "one". This is not standard nor  
spoken Brazilian Portuguese. Brazilian and Iberian Portuguese share the  
same plural form rules.

Mozilla, for instance, uses this rule for localization, but probably the  
rule has never been reviewed by any Brazilian Portuguese-speaker. The  
English Wikipedia also states this rule.


For more information:
http://code.google.com/p/google-web-toolkit/wiki/I18nAnnotations

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[fred]

my use case is for gwt-log
The library supports 7 compile time values. I'd like to ship with one
level so compile time does mulitply by some number >1. Many use
involve users compiling with 2 or 3 levels, which may not include my
library default level. I now have seven gwt XML files to accomodate
the use cases.

Allowing the developer to redefine the allowed valued would be very useful.

Fred
On 9/10/08, BobV <[EMAIL PROTECTED]> wrote:
>
>> - A redefinition of a deferred binding property does not invalidate a
>> previous "set-property" declaration.  If the previous "set-property"
>> declaration is no longer a valid value in the redefined property, it is a
>> static error.
>
> This would be problematic if you wanted to redefine the property to a
> new set of values that is disjoint from the original set.
>
> --
> Bob Vawter
> Google Web Toolkit Team
>
> >
>


-- 
Fred Sauer
[EMAIL PROTECTED]

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

[gwt-contrib] Code Review: gwt-google-apis Maps Convert Duration & Distance to JS Overlays

[Eric Ayers]

Hello Miguel,

It seems like hours since I last sent you a code review, so here's another.
It converts the Distance and Duration objects to JavaScript overlays.

As part of this work, I added a unit test for Directions, as that is where
we encounter these objects.  There is a small change to MarkerTest that gets
rid of a checkstyle warning, and a minor checkstyle fix to
RoutedDirectionsDemo as well.

M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
M  maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java
A  maps/maps/test/com/google/gwt/maps/client/geocode/DirectionsTest.java
D  maps/maps/src/com/google/gwt/maps/client/impl/DurationImpl.java
D  maps/maps/src/com/google/gwt/maps/client/impl/DistanceImpl.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Duration.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Distance.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Route.java
M
maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/RoutedDirectionsDemo.java

-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
M  maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java
A  maps/maps/test/com/google/gwt/maps/client/geocode/DirectionsTest.java
D  maps/maps/src/com/google/gwt/maps/client/impl/DurationImpl.java
D  maps/maps/src/com/google/gwt/maps/client/impl/DistanceImpl.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Duration.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Distance.java
M  maps/maps/src/com/google/gwt/maps/client/geocode/Route.java
M  maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/RoutedDirectionsDemo.java

Index: maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
===
--- maps/maps/test/com/google/gwt/maps/MapsTestSuite.java	(revision 777)
+++ maps/maps/test/com/google/gwt/maps/MapsTestSuite.java	(working copy)
@@ -23,7 +23,7 @@
 import com.google.gwt.maps.client.MapsNotInstalledTest;
 import com.google.gwt.maps.client.StatusCodesTest;
 import com.google.gwt.maps.client.control.ControlTest;
-import com.google.gwt.maps.client.geocode.GeocodeTest;
+import com.google.gwt.maps.client.geocode.DirectionsTest;
 import com.google.gwt.maps.client.geom.BoundsTest;
 import com.google.gwt.maps.client.geom.ProjectionTest;
 import com.google.gwt.maps.client.impl.MinimumMapVersionTest;
@@ -49,7 +49,7 @@
 suite.addTestSuite(MapsNotInstalledTest.class);
 suite.addTestSuite(MapWidgetTest.class);
 suite.addTestSuite(ControlTest.class);
-suite.addTestSuite(GeocodeTest.class);
+suite.addTestSuite(DirectionsTest.class);
 suite.addTestSuite(InfoWindowEventsTest.class);
 suite.addTestSuite(MarkerEventsTest.class);
 suite.addTestSuite(MarkerTest.class);
@@ -62,7 +62,7 @@
 suite.addTestSuite(StatusCodesTest.class);
 suite.addTestSuite(PolyEventsTest.class);
 suite.addTestSuite(ProjectionTest.class);
-//suite.addTestSuite(BoundsTest.class);
+suite.addTestSuite(BoundsTest.class);
 suite.addTestSuite(MinimumMapVersionTest.class);
 return suite;
   }
Index: maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java
===
--- maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java	(revision 777)
+++ maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java	(working copy)
@@ -37,6 +37,7 @@
   /**
* Runs before each test method.
*/
+  @Override
   public void gwtSetUp() {
 TestUtilities.cleanDom();
   }
Index: maps/maps/test/com/google/gwt/maps/client/geocode/DirectionsTest.java
===
--- maps/maps/test/com/google/gwt/maps/client/geocode/DirectionsTest.java	(revision 0)
+++ maps/maps/test/com/google/gwt/maps/client/geocode/DirectionsTest.java	(revision 0)
@@ -0,0 +1,67 @@
+/*
+ * 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.google.gwt.maps.client.geocode;
+
+import com.google.gwt.junit.client.GWTTestCase;
+import com.google.gwt.maps.client.MapWidget;
+
+/*

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Kelly Norton]

On Wed, Sep 10, 2008 at 2:08 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> I think I understand your concern - is it that you don't want to see the
> issue swept under the rug?
>

Yes, that's definitely my concern. And we've lost tests in the past that I
really wish had remained online. They also have a bad compounding effect
where the longer it stays offline the harder it is to bring it back because
someone checks in something that is actually broken and they assume that
since the change passes all tests that everything is good. As a result, we
have a really bad track record of bringing back tests.


>
> I don't know enough right now to fix the test personally, but I do get
> asked about build failures quite often - at least I feel compelled to look
> into them to make sure the test infrastructure is working.
>

And I definitely don't want you having to fix things just because you're
doing the good deed of investigating breakages. I would rather see you
issued a build sheriff badge where you can say: "Build is breaking too much.
The problem seems to be here, can you (person who last edited the test or
generally known to own it) please look into the most appropriate fix."


>
> We had discussions about this particular test failure a few days ago and
> the consensus was that we should comment out the test for the time being and
> bump up the priority on fixing the underlying problem.  I updated issue 864
> before committing the patch, which Joel assigned to John Labanca 2 days ago.
>

 Sounds like you guys have a proper plan to get it back online.

Thanks,
/kel


>
> On Wed, Sep 10, 2008 at 1:49 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:
>
>>
>> On Wed, Sep 10, 2008 at 1:38 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> The failure that's occurring is very similar to the problem described in
>>> #864.  The assumptions of the test or the code we are testing is wrong.  I
>>> don't feel comfortable adding a lot of conditional code in the test when we
>>> already know it is broken.
>>>
>>
>> So why not fix the test if it is operating on faulty assumptions?
>> Disabling tests really should be a last resort and I'm just curious why this
>> one is in last resort state. I feel like if it were in a state where it
>> should be disabled, putting in conditional code shouldn't be that
>> uncomfortable. Also, before disabling it you should also have a plan for
>> getting it back online as well and tie that to a issue tracker item.
>>
>> /kel
>>
>>
>>>
>>> On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]>wrote:
>>>
 Have you considered disabling those tests only for the platforms where
 they are flaky?
 /kel

 On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]>wrote:

> If we already comment out some methods, then LGTM.  Joel doesn't think
> it matters, and I just wanted to be consistent not realizing that we've
> already used both comments and "disabled" methods.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
>
> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> I don't mind doing that.  There are several other methods in ImageTest
>> commented out in the same way I did it.  Want me to do the same for 
>> those?
>>
>>
>> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>
>>> I think we usually just rename the test to "disabledTestXXX" so JUnit
>>> doesn't run it.  Do you mind doing it that way instead, but leaving the
>>> comment?  Otherwise, the next time somebody auto-formats, the method 
>>> will be
>>> formatted like a comment.
>>>
>>> Thanks,
>>> John LaBanca
>>> [EMAIL PROTECTED]
>>>
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]>wrote:
>>>
 Hello John,

 I would like for you to review this patch.

 The testChangeClippedImageToUnclipped() method fails intermittently
 in our continuous build system on Linux hosted mode.  This could be 
 related
 to issues 863 & 864

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

 I've commented out the test, as there are many other tests in this
 TestCase that are commented out due to the same issues.

 M  user/test/com/google/gwt/user/client/ui/ImageTest.java

 --
 Eric Z. Ayers - GWT Team - Atlanta, GA USA
 http://code.google.com/webtoolkit/

>>>
>>>
>>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
> >
>


 --
 If you received this communication by mistake, you are entitled to one
 free ice cream cone on me. Simply print out this email including

[gwt-contrib] Re: one-word review for trivial patch in About.java

[Freeland Abbott]

On Mon, Sep 8, 2008 at 10:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:

> Let me rephrase the question: couldn't we have two sentinel files instead
> of four?  Right now you have two sentinel files for dev/core and two for
> distro-source; each could get by with only one sentinel file whose name
> composited gwt.version with gwt.svnrev.filename.
>

Ah.  Yes, we could have two sentinels rather than four, sorry---I
misunderstood which pair(s) you wanted to fold.


> Detailed review:
>
> At a high level, I'm not sure why you went the route of making things so
> general.  Why does gwt.svninfo need to take parameters to say what props to
> read the values into?  Why not simply always read them into {gwt.svnrev} and
> ${gwt.svnrev.filename}?  For that matter, why does gwt.revfilter need to
> take parameters for those values?  I think both of these should use
> hardcoded property names; the other advantage to this is the ability to
> override these values on the command line and know you've gotten them
> globally.
>

I like macros not to have side effects.  Not strongly enough to contest it,
so the new patch has hardcoded output properties, but that was the
motivation.  (It also would avoid any namespace issues when, inevitably,
this gets cut-and-pasted into the "common".ant.xml of incubator,
gwt-google-apis, etc... though I know those examples have no namespace
issues today anyway.)


> Ideally, those properties could be computed at the top level without
> actually having to remember to call the macro to get them defined, but I'm
> not sure that that's worth the effort.
>

While setting the value should be pretty "fast," if you're working just in
user you may rather not have it set since it won't ever be used.  (We only
have two use instances; I'd swing the other way with a third one.)  So,
yeah, I passed on this one: you need to call  in any
subproject that cares.  That does, however, establish common naming for the
sentinels and creates the sentinel directory, which cleans up some stuff.

The distro-source used to skip sentinels entirely, so there's also some
added exclusion there now that they're creating 'em.



> /dev/common.ant.xml
> - Why do you need to exclude About.properties from both source directories,
> since it will only exist in dev/core?
>

I'd expected common to be common-inclusive-of-core, not just
common-to-plaforms-only.  ("Expected" is maybe too strong; "programmed to
allow for" is perhaps better---but if core included common, then inside core
"src" would be an alias for ${gwt.core.root}/src and would need the
exclusion.)  Today, not needed, so out it goes.


> /distro-source/common.ant.xml
> - Again, 24-25 & 40-41 can be merged to a single sentinel file.
> - 21: What's with the echo here?
>

Yup, and debuggage now gone.

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

Index: common.ant.xml
===
--- common.ant.xml	(revision 3642)
+++ common.ant.xml	(working copy)
@@ -199,10 +199,7 @@
 
   
 
-  
-
-
+  
 
   
@@ -227,7 +224,31 @@
 casesensitive="false" />
   
+  
+  
+  
+   
+   
+
+  
+  
+  
+
+
+
+
   
+  
   
   
 
@@ -236,7 +257,6 @@
   
 
   
-  
 
   
 
Index: dev/common.ant.xml
===
--- dev/common.ant.xml	(revision 3642)
+++ dev/common.ant.xml	(working copy)
@@ -19,8 +19,11 @@
   
 
 
-  
-  
+  
+  
+
+
+  
   
   
   
Index: dev/core/build.xml
===
--- dev/core/build.xml	(revision 3642)
+++ dev/core/build.xml	(working copy)
@@ -57,12 +57,12 @@
 
   
 
-  
-
-
+  
+
   
 
-
+
   
 
   
@@ -76,22 +76,8 @@
 
 
 
-
-
-
-  
-
-
-  
-  
-
-  
-
-
-
 
 
-
 
   
 
@@ -102,8 +88,21 @@
   
 
 
-  
-
+  
+
+
+
+
+
+  
+
+
+  
+  
+
+  
+
+
   
 
   
Index: dev/core/src/com/google/gwt/dev/About.java
===
--- dev/core/src/com/google/gwt/dev/About.java	(revision 3642)
+++ dev/core/src/com/google/gwt/dev/About.java	(working copy)
@@ -32,13 +32,10 @@
 
   public static String GWT_VERSION;
 
-  {
-Class myClass = this.getClass();
-String propsPath = myClass.getName().replace('.', '/').concat(".properties");
+  static {
 Properties props = new Properties();
 try {
-  InputStream instream = myClass.getClassLoader().getResourceAsStream(
-  propsPath);
+  InputStream instream = About.class

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

I think I understand your concern - is it that you don't want to see the
issue swept under the rug?

I don't know enough right now to fix the test personally, but I do get asked
about build failures quite often - at least I feel compelled to look into
them to make sure the test infrastructure is working.

We had discussions about this particular test failure a few days ago and the
consensus was that we should comment out the test for the time being and
bump up the priority on fixing the underlying problem.  I updated issue 864
before committing the patch, which Joel assigned to John Labanca 2 days ago.

On Wed, Sep 10, 2008 at 1:49 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:

>
> On Wed, Sep 10, 2008 at 1:38 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> The failure that's occurring is very similar to the problem described in
>> #864.  The assumptions of the test or the code we are testing is wrong.  I
>> don't feel comfortable adding a lot of conditional code in the test when we
>> already know it is broken.
>>
>
> So why not fix the test if it is operating on faulty assumptions? Disabling
> tests really should be a last resort and I'm just curious why this one is in
> last resort state. I feel like if it were in a state where it should be
> disabled, putting in conditional code shouldn't be that uncomfortable. Also,
> before disabling it you should also have a plan for getting it back online
> as well and tie that to a issue tracker item.
>
> /kel
>
>
>>
>> On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:
>>
>>> Have you considered disabling those tests only for the platforms where
>>> they are flaky?
>>> /kel
>>>
>>> On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>
 If we already comment out some methods, then LGTM.  Joel doesn't think
 it matters, and I just wanted to be consistent not realizing that we've
 already used both comments and "disabled" methods.

 Thanks,
 John LaBanca
 [EMAIL PROTECTED]


 On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> I don't mind doing that.  There are several other methods in ImageTest
> commented out in the same way I did it.  Want me to do the same for those?
>
>
> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>
>> I think we usually just rename the test to "disabledTestXXX" so JUnit
>> doesn't run it.  Do you mind doing it that way instead, but leaving the
>> comment?  Otherwise, the next time somebody auto-formats, the method 
>> will be
>> formatted like a comment.
>>
>> Thanks,
>> John LaBanca
>> [EMAIL PROTECTED]
>>
>>
>>
>> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]>wrote:
>>
>>> Hello John,
>>>
>>> I would like for you to review this patch.
>>>
>>> The testChangeClippedImageToUnclipped() method fails intermittently
>>> in our continuous build system on Linux hosted mode.  This could be 
>>> related
>>> to issues 863 & 864
>>>
>>> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
>>> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>>>
>>> I've commented out the test, as there are many other tests in this
>>> TestCase that are commented out due to the same issues.
>>>
>>> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>>>
>>> --
>>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>>> http://code.google.com/webtoolkit/
>>>
>>
>>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>


 

>>>
>>>
>>> --
>>> If you received this communication by mistake, you are entitled to one
>>> free ice cream cone on me. Simply print out this email including all
>>> relevant SMTP headers and present them at my desk to claim your creamy
>>> treat. We'll have a laugh at my emailing incompetence, and play a game of
>>> ping pong. (offer may not be valid in all States).
>>>
>>
>>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
>
> --
> If you received this communication by mistake, you are entitled to one free
> ice cream cone on me. Simply print out this email including all relevant
> SMTP headers and present them at my desk to claim your creamy treat. We'll
> have a laugh at my emailing incompetence, and play a game of ping pong.
> (offer may not be valid in all States).
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Kelly Norton]

On Wed, Sep 10, 2008 at 1:38 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> The failure that's occurring is very similar to the problem described in
> #864.  The assumptions of the test or the code we are testing is wrong.  I
> don't feel comfortable adding a lot of conditional code in the test when we
> already know it is broken.
>

So why not fix the test if it is operating on faulty assumptions? Disabling
tests really should be a last resort and I'm just curious why this one is in
last resort state. I feel like if it were in a state where it should be
disabled, putting in conditional code shouldn't be that uncomfortable. Also,
before disabling it you should also have a plan for getting it back online
as well and tie that to a issue tracker item.

/kel


>
> On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:
>
>> Have you considered disabling those tests only for the platforms where
>> they are flaky?
>> /kel
>>
>> On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>
>>> If we already comment out some methods, then LGTM.  Joel doesn't think it
>>> matters, and I just wanted to be consistent not realizing that we've already
>>> used both comments and "disabled" methods.
>>>
>>> Thanks,
>>> John LaBanca
>>> [EMAIL PROTECTED]
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
 I don't mind doing that.  There are several other methods in ImageTest
 commented out in the same way I did it.  Want me to do the same for those?


 On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:

> I think we usually just rename the test to "disabledTestXXX" so JUnit
> doesn't run it.  Do you mind doing it that way instead, but leaving the
> comment?  Otherwise, the next time somebody auto-formats, the method will 
> be
> formatted like a comment.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
>
>
> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello John,
>>
>> I would like for you to review this patch.
>>
>> The testChangeClippedImageToUnclipped() method fails intermittently in
>> our continuous build system on Linux hosted mode.  This could be related 
>> to
>> issues 863 & 864
>>
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>>
>> I've commented out the test, as there are many other tests in this
>> TestCase that are commented out due to the same issues.
>>
>> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>


 --
 Eric Z. Ayers - GWT Team - Atlanta, GA USA
 http://code.google.com/webtoolkit/

>>>
>>>
>>> >>>
>>>
>>
>>
>> --
>> If you received this communication by mistake, you are entitled to one
>> free ice cream cone on me. Simply print out this email including all
>> relevant SMTP headers and present them at my desk to claim your creamy
>> treat. We'll have a laugh at my emailing incompetence, and play a game of
>> ping pong. (offer may not be valid in all States).
>>
>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
If you received this communication by mistake, you are entitled to one free
ice cream cone on me. Simply print out this email including all relevant
SMTP headers and present them at my desk to claim your creamy treat. We'll
have a laugh at my emailing incompetence, and play a game of ping pong.
(offer may not be valid in all States).

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

Committed as r3644.

On Wed, Sep 10, 2008 at 1:38 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> The failure that's occurring is very similar to the problem described in
> #864.  The assumptions of the test or the code we are testing is wrong.  I
> don't feel comfortable adding a lot of conditional code in the test when we
> already know it is broken.
>
>
> On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:
>
>> Have you considered disabling those tests only for the platforms where
>> they are flaky?
>> /kel
>>
>> On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>
>>> If we already comment out some methods, then LGTM.  Joel doesn't think it
>>> matters, and I just wanted to be consistent not realizing that we've already
>>> used both comments and "disabled" methods.
>>>
>>> Thanks,
>>> John LaBanca
>>> [EMAIL PROTECTED]
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
 I don't mind doing that.  There are several other methods in ImageTest
 commented out in the same way I did it.  Want me to do the same for those?


 On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:

> I think we usually just rename the test to "disabledTestXXX" so JUnit
> doesn't run it.  Do you mind doing it that way instead, but leaving the
> comment?  Otherwise, the next time somebody auto-formats, the method will 
> be
> formatted like a comment.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
>
>
> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello John,
>>
>> I would like for you to review this patch.
>>
>> The testChangeClippedImageToUnclipped() method fails intermittently in
>> our continuous build system on Linux hosted mode.  This could be related 
>> to
>> issues 863 & 864
>>
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>>
>> I've commented out the test, as there are many other tests in this
>> TestCase that are commented out due to the same issues.
>>
>> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>


 --
 Eric Z. Ayers - GWT Team - Atlanta, GA USA
 http://code.google.com/webtoolkit/

>>>
>>>
>>> >>>
>>>
>>
>>
>> --
>> If you received this communication by mistake, you are entitled to one
>> free ice cream cone on me. Simply print out this email including all
>> relevant SMTP headers and present them at my desk to claim your creamy
>> treat. We'll have a laugh at my emailing incompetence, and play a game of
>> ping pong. (offer may not be valid in all States).
>>
>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

The failure that's occurring is very similar to the problem described in
#864.  The assumptions of the test or the code we are testing is wrong.  I
don't feel comfortable adding a lot of conditional code in the test when we
already know it is broken.

On Wed, Sep 10, 2008 at 1:28 PM, Kelly Norton <[EMAIL PROTECTED]> wrote:

> Have you considered disabling those tests only for the platforms where they
> are flaky?
> /kel
>
> On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]> wrote:
>
>> If we already comment out some methods, then LGTM.  Joel doesn't think it
>> matters, and I just wanted to be consistent not realizing that we've already
>> used both comments and "disabled" methods.
>>
>> Thanks,
>> John LaBanca
>> [EMAIL PROTECTED]
>>
>>
>> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> I don't mind doing that.  There are several other methods in ImageTest
>>> commented out in the same way I did it.  Want me to do the same for those?
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>
 I think we usually just rename the test to "disabledTestXXX" so JUnit
 doesn't run it.  Do you mind doing it that way instead, but leaving the
 comment?  Otherwise, the next time somebody auto-formats, the method will 
 be
 formatted like a comment.

 Thanks,
 John LaBanca
 [EMAIL PROTECTED]



 On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hello John,
>
> I would like for you to review this patch.
>
> The testChangeClippedImageToUnclipped() method fails intermittently in
> our continuous build system on Linux hosted mode.  This could be related 
> to
> issues 863 & 864
>
> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>
> I've commented out the test, as there are many other tests in this
> TestCase that are commented out due to the same issues.
>
> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>


>>>
>>>
>>> --
>>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>>> http://code.google.com/webtoolkit/
>>>
>>
>>
>> >>
>>
>
>
> --
> If you received this communication by mistake, you are entitled to one free
> ice cream cone on me. Simply print out this email including all relevant
> SMTP headers and present them at my desk to claim your creamy treat. We'll
> have a laugh at my emailing incompetence, and play a game of ping pong.
> (offer may not be valid in all States).
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] [google-web-toolkit commit] r3644 - trunk/user/test/com/google/gwt/user/client/ui

[codesite-noreply]

Author: [EMAIL PROTECTED]
Date: Wed Sep 10 10:36:08 2008
New Revision: 3644

Modified:
trunk/user/test/com/google/gwt/user/client/ui/ImageTest.java

Log:
Commented out testChangeClippedImageToUnclipped() because it fails
intermittently in the Linux continuous build in hosted mode.

Patch by: zundel
Review by: jlabanca


Modified: trunk/user/test/com/google/gwt/user/client/ui/ImageTest.java
==
--- trunk/user/test/com/google/gwt/user/client/ui/ImageTest.java
(original)
+++ trunk/user/test/com/google/gwt/user/client/ui/ImageTest.javaWed Sep 
10  
10:36:08 2008
@@ -83,6 +83,8 @@
/**
 * Tests the transition from the clipped state to the unclipped state.
 */
+  /* This test is commented out because of issue #863 & #864
+ It fails intermittently in linux hosted mode tests.
public void testChangeClippedImageToUnclipped() {
  final Image image = new Image("counting-forwards.png",
  12, 13, 8, 8);
@@ -115,7 +117,8 @@

  delayTestFinish(5000);
}
-
+  */
+
/**
 *  Tests the creation of an image in unclipped mode
 */

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Kelly Norton]

Have you considered disabling those tests only for the platforms where they
are flaky?
/kel

On Wed, Sep 10, 2008 at 1:26 PM, John LaBanca <[EMAIL PROTECTED]> wrote:

> If we already comment out some methods, then LGTM.  Joel doesn't think it
> matters, and I just wanted to be consistent not realizing that we've already
> used both comments and "disabled" methods.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
>
> On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> I don't mind doing that.  There are several other methods in ImageTest
>> commented out in the same way I did it.  Want me to do the same for those?
>>
>>
>> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>
>>> I think we usually just rename the test to "disabledTestXXX" so JUnit
>>> doesn't run it.  Do you mind doing it that way instead, but leaving the
>>> comment?  Otherwise, the next time somebody auto-formats, the method will be
>>> formatted like a comment.
>>>
>>> Thanks,
>>> John LaBanca
>>> [EMAIL PROTECTED]
>>>
>>>
>>>
>>> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
 Hello John,

 I would like for you to review this patch.

 The testChangeClippedImageToUnclipped() method fails intermittently in
 our continuous build system on Linux hosted mode.  This could be related to
 issues 863 & 864

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

 I've commented out the test, as there are many other tests in this
 TestCase that are commented out due to the same issues.

 M  user/test/com/google/gwt/user/client/ui/ImageTest.java

 --
 Eric Z. Ayers - GWT Team - Atlanta, GA USA
 http://code.google.com/webtoolkit/

>>>
>>>
>>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
> >
>


-- 
If you received this communication by mistake, you are entitled to one free
ice cream cone on me. Simply print out this email including all relevant
SMTP headers and present them at my desk to claim your creamy treat. We'll
have a laugh at my emailing incompetence, and play a game of ping pong.
(offer may not be valid in all States).

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[John LaBanca]

If we already comment out some methods, then LGTM.  Joel doesn't think it
matters, and I just wanted to be consistent not realizing that we've already
used both comments and "disabled" methods.

Thanks,
John LaBanca
[EMAIL PROTECTED]


On Wed, Sep 10, 2008 at 1:19 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> I don't mind doing that.  There are several other methods in ImageTest
> commented out in the same way I did it.  Want me to do the same for those?
>
>
> On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]> wrote:
>
>> I think we usually just rename the test to "disabledTestXXX" so JUnit
>> doesn't run it.  Do you mind doing it that way instead, but leaving the
>> comment?  Otherwise, the next time somebody auto-formats, the method will be
>> formatted like a comment.
>>
>> Thanks,
>> John LaBanca
>> [EMAIL PROTECTED]
>>
>>
>>
>> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> Hello John,
>>>
>>> I would like for you to review this patch.
>>>
>>> The testChangeClippedImageToUnclipped() method fails intermittently in
>>> our continuous build system on Linux hosted mode.  This could be related to
>>> issues 863 & 864
>>>
>>> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
>>> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>>>
>>> I've commented out the test, as there are many other tests in this
>>> TestCase that are commented out due to the same issues.
>>>
>>> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>>>
>>> --
>>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>>> http://code.google.com/webtoolkit/
>>>
>>
>>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>

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

[gwt-contrib] Re: Code Review Request: prevent spurious wiki links in JRE emulation docs

[Keith Platfoot]

Thanks, Scott.  Committed as r3643.
Keith

On Fri, Aug 29, 2008 at 6:48 PM, Scott Blum <[EMAIL PROTECTED]> wrote:

> Looks fine to me.  Bruce might also want to take a look.
>
>
> On Fri, Aug 29, 2008 at 3:56 PM, Keith Platfoot <[EMAIL PROTECTED]>wrote:
>
>> Hi Scott,
>>
>> When you have some time, could you review this tiny patch to the JRE
>> emulation documentation generator?  I've modified the header of the
>> generated wiki output per Bruce's request, and have also added a bit of code
>> to WikiDoclet to prefix any wiki-words (WordsLikeThis) with '!' so Google
>> Code doesn't try to automatically turn them into links.
>> Affected files:
>>
>> M  doc\src\RefJreHeader.wiki
>> M  build-tools\doctool\src\com\google\doctool\custom\WikiDoclet.java
>>
>> Thanks,
>>
>> Keith
>>
>>
>

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

[gwt-contrib] [google-web-toolkit commit] r3643 - in trunk: build-tools/doctool/src/com/google/doctool/custom doc/src

[codesite-noreply]

Author: [EMAIL PROTECTED]
Date: Wed Sep 10 10:23:51 2008
New Revision: 3643

Modified:
trunk/build-tools/doctool/src/com/google/doctool/custom/WikiDoclet.java
trunk/doc/src/RefJreHeader.wiki

Log:
Changed the header of the generated JRE emulation wiki doc and modified the  
WikiDoclet to prefix any wiki-words (WordsLikeThis) with '!' so Google Code  
doesn't try to automatically turn them into links.

Review by: scottb


Modified:  
trunk/build-tools/doctool/src/com/google/doctool/custom/WikiDoclet.java
==
--- trunk/build-tools/doctool/src/com/google/doctool/custom/WikiDoclet.java 
 
(original)
+++ trunk/build-tools/doctool/src/com/google/doctool/custom/WikiDoclet.java 
 
Wed Sep 10 10:23:51 2008
@@ -33,6 +33,7 @@
  import java.util.Collection;
  import java.util.Comparator;
  import java.util.Iterator;
+import java.util.regex.Pattern;

  /**
   * A doclet for using producing wiki output listing the specified classes  
and
@@ -40,12 +41,18 @@
   */
  public class WikiDoclet {

+  private static final String JAVADOC_URL  
= "http://java.sun.com/j2se/1.5.0/docs/api/";;
private static final String OPT_WKHEADER = "-wkhead";
private static final String OPT_WKOUT = "-wkout";
-  private static final String JAVADOC_URL  
= "http://java.sun.com/j2se/1.5.0/docs/api/";;

private static WikiDoclet sWikiDoclet;

+  /*
+   * Finds any compound Pascal-cased words (LikeThisExample), which Google  
Code
+   * will try to interpret as wiki links.
+   */
+  private static final Pattern wikiWordPattern =  
Pattern.compile("\\b([A-Z]+\\w+[A-Z]+\\w*)\\b");
+
public static int optionLength(String option) {
  if (option.equals(OPT_WKOUT)) {
return 2;
@@ -67,6 +74,11 @@
  return getDoclet().analyzeOptions(options, reporter);
}

+  private static String escapeWikiWords(String text) {
+// Prefix each match with '!' to suppress automatic wiki-linking
+return wikiWordPattern.matcher(text).replaceAll("!$1");
+  }
+
private static WikiDoclet getDoclet() {
  if (sWikiDoclet == null) {
sWikiDoclet = new WikiDoclet();
@@ -101,7 +113,7 @@
  Iterator iter = members.iterator();
  while (iter.hasNext()) {
ExecutableMemberDoc member = iter.next();
-  buffer.append(member.name() + member.flatSignature());
+  buffer.append(escapeWikiWords(member.name() +  
member.flatSignature()));
if (iter.hasNext()) {
  buffer.append(", ");
}

Modified: trunk/doc/src/RefJreHeader.wiki
==
--- trunk/doc/src/RefJreHeader.wiki (original)
+++ trunk/doc/src/RefJreHeader.wiki Wed Sep 10 10:23:51 2008
@@ -1,4 +1,4 @@
-#summary JRE Emulation Library
+#summary The JRE types and methods that GWT emulates.

  =JRE Emulation=
  Google Web Toolkit includes a library that emulates a subset of the Java  
runtime library. The list below shows the set of JRE types and methods that  
GWT can translate automatically. Note that in some cases, only a subset of  
methods is supported for a given type.

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[BobV]

> - A redefinition of a deferred binding property does not invalidate a
> previous "set-property" declaration.  If the previous "set-property"
> declaration is no longer a valid value in the redefined property, it is a
> static error.

This would be problematic if you wanted to redefine the property to a
new set of values that is disjoint from the original set.

-- 
Bob Vawter
Google Web Toolkit Team

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

I don't mind doing that.  There are several other methods in ImageTest
commented out in the same way I did it.  Want me to do the same for those?

On Wed, Sep 10, 2008 at 1:13 PM, John LaBanca <[EMAIL PROTECTED]> wrote:

> I think we usually just rename the test to "disabledTestXXX" so JUnit
> doesn't run it.  Do you mind doing it that way instead, but leaving the
> comment?  Otherwise, the next time somebody auto-formats, the method will be
> formatted like a comment.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>
>
>
> On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello John,
>>
>> I would like for you to review this patch.
>>
>> The testChangeClippedImageToUnclipped() method fails intermittently in our
>> continuous build system on Linux hosted mode.  This could be related to
>> issues 863 & 864
>>
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
>> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>>
>> I've commented out the test, as there are many other tests in this
>> TestCase that are commented out due to the same issues.
>>
>> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>


-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: Comment out flaky test in ImageTest

[John LaBanca]

I think we usually just rename the test to "disabledTestXXX" so JUnit
doesn't run it.  Do you mind doing it that way instead, but leaving the
comment?  Otherwise, the next time somebody auto-formats, the method will be
formatted like a comment.

Thanks,
John LaBanca
[EMAIL PROTECTED]


On Wed, Sep 10, 2008 at 1:10 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hello John,
>
> I would like for you to review this patch.
>
> The testChangeClippedImageToUnclipped() method fails intermittently in our
> continuous build system on Linux hosted mode.  This could be related to
> issues 863 & 864
>
> http://code.google.com/p/google-web-toolkit/issues/detail?id=863
> http://code.google.com/p/google-web-toolkit/issues/detail?id=864
>
> I've commented out the test, as there are many other tests in this TestCase
> that are commented out due to the same issues.
>
> M  user/test/com/google/gwt/user/client/ui/ImageTest.java
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>

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

[gwt-contrib] Code Review: Comment out flaky test in ImageTest

[Eric Ayers]

Hello John,

I would like for you to review this patch.

The testChangeClippedImageToUnclipped() method fails intermittently in our
continuous build system on Linux hosted mode.  This could be related to
issues 863 & 864

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

I've commented out the test, as there are many other tests in this TestCase
that are commented out due to the same issues.

M  user/test/com/google/gwt/user/client/ui/ImageTest.java

-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

Index: user/test/com/google/gwt/user/client/ui/ImageTest.java
===
--- user/test/com/google/gwt/user/client/ui/ImageTest.java	(revision 3642)
+++ user/test/com/google/gwt/user/client/ui/ImageTest.java	(working copy)
@@ -83,6 +83,8 @@
   /**
* Tests the transition from the clipped state to the unclipped state.
*/
+  /* This test is commented out because of issue #863 & #864
+ It fails intermittently in linux hosted mode tests.
   public void testChangeClippedImageToUnclipped() {
 final Image image = new Image("counting-forwards.png",
 12, 13, 8, 8);
@@ -115,7 +117,8 @@
 
 delayTestFinish(5000);
   }
-
+  */
+  
   /**
*  Tests the creation of an image in unclipped mode
*/

[gwt-contrib] Re: Code Review: gwt-google-apis Maps convert PolyXXXOptions to JavaScript overlays

[Miguel Méndez]

LGTM

On Wed, Sep 10, 2008 at 12:52 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

>
>
> On Wed, Sep 10, 2008 at 9:48 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:
>
>>
>> On Tue, Sep 9, 2008 at 3:54 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> Hello Miguel,
>>>
>>> I've combined the conversion of several classes into one patch because of
>>> the overlap.
>>>
>>>  - PolylineOptions
>>>  - PolygonOptions
>>>  - Fixed constructor name in PolyStyleOptions.java
>>>  - Fixed references to above.
>>>  - Changed some references to PolylineOptions to EncodedPolyline... that
>>> was weird!
>>>
>>> M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
>>>
>> LG
>>
>>
>>>
>>> M
>>> maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
>>>
>> LG - consider adding a newInstance(boolean clickable) method.
>>
> done.
>
>
>>
>>>
>>> A
>>> maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
>>>
>> LG - This is one place where we could provide an additional newInstance
>> overload to specify clickable and geodesic.
>>
> created newInstance(boolean clickable) and newInstance(boolean clickable,
> boolean geodesic)
>
>>
>>
>>>
>>> M
>>> maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
>>>
>> LG
>>
>>
>>>
>>> M
>>> maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
>>>
>> LG
>>
>>
>>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>>>
>> LG
>>
>>
>>>  M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
>>>
>> LG
>>
>>
>>>  D
>>> maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
>>>
>> LG
>>
>>
>>>  M
>>> maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
>>>
>> LG - Is this an overlay conversion candidate?
>>
>> D
>>> maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
>>>
>> LG
>>
>>
>>> D
>>> maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
>>>
>> LG
>>
>>
>>>  M
>>> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
>>>
>> LG
>>
>> M
>>> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java
>>>
>> LG
>>
>>
>>>
>>>
>>> --
>>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>>> http://code.google.com/webtoolkit/
>>>
>>
>>
>>
>> --
>> Miguel
>>
>
> I've gone ahead and committed this patch as r774, but attached it in case
> you feel it needs further review. I Modified the tests a bit, adding a new
> PolylineTest class.
>
> M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
> M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
> M  maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
> A
> maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
> M
> maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
> M
> maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
> D
> maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
> M
> maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
> D
> maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
> D
> maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
> M
> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
> M
> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Bounds to JavaScript overlay

[Eric Ayers]

So the word from Pam is that using ints should be fine.

On Wed, Sep 10, 2008 at 11:12 AM, Pamela Fox <[EMAIL PROTECTED]> wrote:

> +appleton, who can comment if he disagres
>
> Both Bounds and Point are generally used to represent pixel coordinates
> (and worded that way in the reference), so since pixels can't be divided,
> using int should be fine. I don't think I've ever tried to pass a fraction
> to them myself.
>
> Note: We do have a special GScreenPoint class which does take fractions and
> is used by screen 
> overlays:
> http://code.google.com/apis/maps/documentation/reference.html#GScreenPoint
>
>
> On Wed, Sep 10, 2008 at 7:12 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> [-GWTC, +pamelafox]
>>
>> Hello Pamela,
>>
>>   Miguel & I wanted to check with you to see if you thought it important
>> that Bounds() and Point() should support fractional values.  I know it makes
>> no difference in JS, there being no integer type, but it matters in Java and
>> how users expect the API to work. The documentation says these are for pixel
>> values, so I assumed they should be integers.  To date, we have modelled the
>> inputs and outputs from these methods as 'int' types in Java.  I looked
>> through geometry.js but it wasn't entirely enlightening.  The safest thing
>> we can do is to pass doubles through, but I think this would be a bit
>> confusing to users (what happens if you position a control at .5 pixels?
>> Anti-aliasing?)  Can you help us resolve this?
>>
>> Thanks,
>> -Eric.
>>
>> -- Forwarded message --
>> From: Miguel Méndez <[EMAIL PROTECTED]>
>> Date: Wed, Sep 10, 2008 at 9:59 AM
>> Subject: Re: Code Review: gwt-google-apis Maps Convert Bounds to
>> JavaScript overlay
>> To: Eric Ayers <[EMAIL PROTECTED]>
>> Cc: GWTcontrib 
>>
>>
>>
>> On Wed, Sep 10, 2008 at 9:43 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> On Wed, Sep 10, 2008 at 9:13 AM, Miguel Méndez <[EMAIL PROTECTED]>wrote:
>>>
 On Mon, Sep 8, 2008 at 5:37 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> M  maps/maps/src/com/google/gwt/maps/client/geom/Bounds.java
>
 37 - This Bounds construction is not part of the Maps API proper and it
 is only called from our test code.  Do we still need it?

>>> I added this as a convenience, because I was tired of constructing
>>> JsArrays over and over in the test code.  It results in compiled code just
>>> as tight or tighter (if you don't already have Point objects made).  We
>>> could add a 3rd constructor with 2 point objects.
>>>
>>
>> Sounds like it should be in the test code then.  If anything you could
>> create a variadic method to take Point arguments and return a
>> JsArray.  I would not worry about a third constructor for now.
>>
>>
>>>
>>>
>>>
 105, 114, 123, 132 - Should get[Max|Min][X|Y] just return doubles?  The
 JS API just states that this is a Number however the documentation would
 lead you to believe that it is really an integer value.  It seems like the
 safest thing to do would be to use a double.  Maybe Pam could shed some
 light?  This would call into question the newPoint method.

>>>
>>> This sounds familiar - have we been through this before?  Along with
>>> Point, this object represents pixels on the screen.  Of course, JS doesn't
>>> have an integer type, but I see in the JS code that pixel values (stored in
>>> the Bounds object) are looped through with for loops with operators like x++
>>> and y--, indicating to me they are intended to be integers.
>>>
>>>
>>
>> We have been through this before, but it seems that every instance in the
>> maps API is unique.  ++ and -- are valid for floating point also it might be
>> more illuminating to look at the terminating condition for the loop.  Better
>> yet, just double check with Pam.
>>
>> --
>> Miguel
>>
>>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>


-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps convert PolyXXXOptions to JavaScript overlays

[Eric Ayers]

On Wed, Sep 10, 2008 at 9:48 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

>
> On Tue, Sep 9, 2008 at 3:54 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello Miguel,
>>
>> I've combined the conversion of several classes into one patch because of
>> the overlap.
>>
>>  - PolylineOptions
>>  - PolygonOptions
>>  - Fixed constructor name in PolyStyleOptions.java
>>  - Fixed references to above.
>>  - Changed some references to PolylineOptions to EncodedPolyline... that
>> was weird!
>>
>> M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
>>
> LG
>
>
>>
>> M
>> maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
>>
> LG - consider adding a newInstance(boolean clickable) method.
>
done.


>
>>
>> A
>> maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
>>
> LG - This is one place where we could provide an additional newInstance
> overload to specify clickable and geodesic.
>
created newInstance(boolean clickable) and newInstance(boolean clickable,
boolean geodesic)

>
>
>>
>> M
>> maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
>>
> LG
>
>
>>
>> M
>> maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
>>
> LG
>
>
>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>>
> LG
>
>
>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
>>
> LG
>
>
>> D
>> maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
>>
> LG
>
>
>>  M
>> maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
>>
> LG - Is this an overlay conversion candidate?
>
> D
>> maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
>>
> LG
>
>
>> D
>> maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
>>
> LG
>
>
>>  M
>> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
>>
> LG
>
> M
>> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java
>>
> LG
>
>
>>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
>
> --
> Miguel
>

I've gone ahead and committed this patch as r774, but attached it in case
you feel it needs further review. I Modified the tests a bit, adding a new
PolylineTest class.

M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
A  maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
M
maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
D
maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
M
maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
D  maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
D
maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
M
maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
M
maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java


-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
A  maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
D  maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
M  maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
D  maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
D  maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
M  maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
M  maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java

Index: maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
===
--- maps/maps/test/com/google/gwt/maps/MapsTestSuite.java	(revision 773)
+++ maps/maps/test/com/google/gwt/maps/MapsTestSuite.java	(working copy)

[gwt-contrib] Re: RR : Allow to redefine an existing property

[BobV]

It was the issue of repeated "restrict range of values" tags that made
me decide that define-property had a far more obvious semantic when
it's repeated.  And, as a convenience to developers, you can add
additional property values with the one tag.

-- 
Bob Vawter
Google Web Toolkit Team

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[John Tamplin]

On Wed, Sep 10, 2008 at 12:10 PM, Bruce Johnson <[EMAIL PROTECTED]> wrote:

> @Scott: Basically, why exactly don't you like my reasoning? Let me be more
> concise to see if you find it more sensible:
> define-property: introduces a property and its initial set of allowed
> values; it is illegal to re-define an existing property, which is a useful
> guard against collisions
> extend-property: add new values to an existing property; we statically
> check that you are extending a known property
> set-property: restricts the value of an existing property to one or more
> particular values; we statically check that you are setting a known property
> to known values
>

set-property seems overloaded to me -- in the existing case, it can be used
to replace a property provider by choosing a particular value while not
affecting the set of legal property values.  Here, you are suggesting it do
both if you supply multiple values.  If you don't like allowing
redefinitions with define-property, I would suggest a different tag, like
remove-property or restrict-property which changes the set of available
values.  This would keep it totally orthogonal to set-property, leaving it
to only mean setting the active value of the property.

As an example of where it makes a difference, LocaleInfoGenerator cares
about the list of legal property values separately from which one is being
compiled for currently -- if set-values replaced the set of legal property
values, then the output would change even if you simply wanted to only
compile a subset of permutations without affecting the generated code.


>
> Beauty, no? The only other thing I could imagine wanting to add (if there's
> a true need) is .
>
> On Wed, Sep 10, 2008 at 11:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:
>
>> Honestly, I'm a little torn about the interaction between restricting the
>> range of properties and an explicit "set-property" call.  It feels like the
>> task of trying to restrict the range of valid properties shouldn't clobber a
>> previous pinpointing of exactly what property you want, because they seem
>> like different operations.
>> I think I'm slightly in favor of Bob's original proposal, with one
>> modification:
>>
>> - A redefinition of a deferred binding property does not invalidate a
>> previous "set-property" declaration.  If the previous "set-property"
>> declaration is no longer a valid value in the redefined property, it is a
>> static error.
>>
>>
>> On Wed, Sep 10, 2008 at 10:25 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:
>>
>>> Sorry, typo. See correction below.
>>>
>>> On Wed, Sep 10, 2008 at 10:24 AM, Bruce Johnson <[EMAIL PROTECTED]>wrote:
>>>
 I propose instead of allow  to have a "values" attribute
 as a mutually-exclusive alternative to the "value" attribute:

>>>
>>> I propose instead to allow  to have a "values" attribute...
>>> (ditto above)
>>>
>>>
>>
>
> >
>


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

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Bruce Johnson]

I basically think we don't want to support the use case you describe.
Normally, all the rules are "the last one to specify something wins", so it
would be a lot clearer to honor the bottom-most  than to
subtly combine them.
Plus, a developer would quickly realize that something's wrong via SOYC,
right? (wink, wink)

On Wed, Sep 10, 2008 at 12:21 PM, Scott Blum <[EMAIL PROTECTED]> wrote:

> Why didn't you just put it that way the first time? :P
> But seriously, my perhaps-silly thought was what about this sort of case
> (assume ultimate lexical order):
>
> Module 1 (say, UserAgent.gwt.xml)
>  values="ie6,gecko,gecko1_8,safari,opera"/>
>
> Module 2 (a module that I'm using to explicitly target just Safari; let's
> pretend its IPhone.gwt.xml):
> 
>
> Module 3 (prevent building for old Mozilla):
> 
>
> I don't think this use case can be supported with your proposal, since the
> second set-property will clobber the first.  Maybe this is a silly point,
> because we could insist the developer change the lexical order, although I
> could image cases where that's slightly cumbersome. This would be
> supportable if the third module could do a "restrict-property" that didn't
> clobber the explicitly set value.
>
> Again, my thinking could be quite silly here.
>
> On Wed, Sep 10, 2008 at 12:10 PM, Bruce Johnson <[EMAIL PROTECTED]> wrote:
>
>> @Scott: Basically, why exactly don't you like my reasoning? Let me be more
>> concise to see if you find it more sensible:
>> define-property: introduces a property and its initial set of allowed
>> values; it is illegal to re-define an existing property, which is a useful
>> guard against collisions
>> extend-property: add new values to an existing property; we statically
>> check that you are extending a known property
>> set-property: restricts the value of an existing property to one or more
>> particular values; we statically check that you are setting a known property
>> to known values
>>
>> Beauty, no? The only other thing I could imagine wanting to add (if
>> there's a true need) is .
>>
>>

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Scott Blum]

Why didn't you just put it that way the first time? :P
But seriously, my perhaps-silly thought was what about this sort of case
(assume ultimate lexical order):

Module 1 (say, UserAgent.gwt.xml)


Module 2 (a module that I'm using to explicitly target just Safari; let's
pretend its IPhone.gwt.xml):


Module 3 (prevent building for old Mozilla):


I don't think this use case can be supported with your proposal, since the
second set-property will clobber the first.  Maybe this is a silly point,
because we could insist the developer change the lexical order, although I
could image cases where that's slightly cumbersome. This would be
supportable if the third module could do a "restrict-property" that didn't
clobber the explicitly set value.

Again, my thinking could be quite silly here.

On Wed, Sep 10, 2008 at 12:10 PM, Bruce Johnson <[EMAIL PROTECTED]> wrote:

> @Scott: Basically, why exactly don't you like my reasoning? Let me be more
> concise to see if you find it more sensible:
> define-property: introduces a property and its initial set of allowed
> values; it is illegal to re-define an existing property, which is a useful
> guard against collisions
> extend-property: add new values to an existing property; we statically
> check that you are extending a known property
> set-property: restricts the value of an existing property to one or more
> particular values; we statically check that you are setting a known property
> to known values
>
> Beauty, no? The only other thing I could imagine wanting to add (if there's
> a true need) is .
>
>

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

[gwt-contrib] Re: RR : Three new RPC design docs

[Bruce Johnson]

The purpose of GWT RPC is to be as fast as possible for Java-to-Java
communication, so it's hard to imagine ever compromising that goal for
cross-language interop.
For interop, use RequestBuilder + JSON or XML or maybe, as John says, a
future protobuff implementation.

On Wed, Sep 10, 2008 at 3:52 AM, Johan Rydberg <[EMAIL PROTECTED]>wrote:

>
>
> > This document is more about how we might revamp the RPC payload format
> > to improve deserialization speed, eliminate recursive behavior, and
> > provide a well-supported mechanism for custom serialization:
> >   http://code.google.com/p/google-web-toolkit/wiki/RpcDirectEval
>
> I just say that I would like to see a format that doesn't make it harder
> for non-Java backend implementations, than it already is.
>
> I can think of these right now:
>
>   http://code.google.com/p/python-gwt-rpc/
>   http://code.google.com/p/gwtphp/
>   http://code.google.com/p/twisted-gwt/  (my project)
>
> (Not suggesting that the proposed format is anyway harder to encode to).
>
> >
>

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Bruce Johnson]

@Scott: Basically, why exactly don't you like my reasoning? Let me be more
concise to see if you find it more sensible:
define-property: introduces a property and its initial set of allowed
values; it is illegal to re-define an existing property, which is a useful
guard against collisions
extend-property: add new values to an existing property; we statically check
that you are extending a known property
set-property: restricts the value of an existing property to one or more
particular values; we statically check that you are setting a known property
to known values

Beauty, no? The only other thing I could imagine wanting to add (if there's
a true need) is .

On Wed, Sep 10, 2008 at 11:56 AM, Scott Blum <[EMAIL PROTECTED]> wrote:

> Honestly, I'm a little torn about the interaction between restricting the
> range of properties and an explicit "set-property" call.  It feels like the
> task of trying to restrict the range of valid properties shouldn't clobber a
> previous pinpointing of exactly what property you want, because they seem
> like different operations.
> I think I'm slightly in favor of Bob's original proposal, with one
> modification:
>
> - A redefinition of a deferred binding property does not invalidate a
> previous "set-property" declaration.  If the previous "set-property"
> declaration is no longer a valid value in the redefined property, it is a
> static error.
>
>
> On Wed, Sep 10, 2008 at 10:25 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:
>
>> Sorry, typo. See correction below.
>>
>> On Wed, Sep 10, 2008 at 10:24 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:
>>
>>> I propose instead of allow  to have a "values" attribute as
>>> a mutually-exclusive alternative to the "value" attribute:
>>>
>>
>> I propose instead to allow  to have a "values" attribute...
>> (ditto above)
>>
>>
>

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

[gwt-contrib] [google-web-toolkit commit] r3642 - trunk/user/src/com/google/gwt/user/client/ui

[codesite-noreply]

Author: [EMAIL PROTECTED]
Date: Wed Sep 10 08:58:07 2008
New Revision: 3642

Modified:
trunk/user/src/com/google/gwt/user/client/ui/TabBar.java

Log:
Added styles gwt-TabBarFirst-wrapper and gwt-TabBarRest-wrapper to the  
TabBar.

Patch by: jlabanca
Review by: ecc
Issue: 2869



Modified: trunk/user/src/com/google/gwt/user/client/ui/TabBar.java
==
--- trunk/user/src/com/google/gwt/user/client/ui/TabBar.java(original)
+++ trunk/user/src/com/google/gwt/user/client/ui/TabBar.javaWed Sep 10  
08:58:07 2008
@@ -28,7 +28,11 @@
   * 
   * .gwt-TabBar { the tab bar itself }
   * .gwt-TabBar .gwt-TabBarFirst { the left edge of the bar }
+ * .gwt-TabBar .gwt-TabBarFirst-wrapper { table cell around the left  
edge }
+ * 
   * .gwt-TabBar .gwt-TabBarRest { the right edge of the bar }
+ * .gwt-TabBar .gwt-TabBarRest-wrapper { table cell around the right  
edge }
+ * 
   * .gwt-TabBar .gwt-TabBarItem { unselected tabs }
   * .gwt-TabBar .gwt-TabBarItem-wrapper { table cell around tab }
   * .gwt-TabBar .gwt-TabBarItem-selected { additional style for selected
@@ -122,6 +126,10 @@
  first.setHeight("100%");
  panel.setCellHeight(first, "100%");
  panel.setCellWidth(rest, "100%");
+setStyleName(first.getElement().getParentElement(),
+"gwt-TabBarFirst-wrapper");
+setStyleName(rest.getElement().getParentElement(),
+"gwt-TabBarRest-wrapper");
}

/**

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Scott Blum]

Honestly, I'm a little torn about the interaction between restricting the
range of properties and an explicit "set-property" call.  It feels like the
task of trying to restrict the range of valid properties shouldn't clobber a
previous pinpointing of exactly what property you want, because they seem
like different operations.
I think I'm slightly in favor of Bob's original proposal, with one
modification:

- A redefinition of a deferred binding property does not invalidate a
previous "set-property" declaration.  If the previous "set-property"
declaration is no longer a valid value in the redefined property, it is a
static error.

On Wed, Sep 10, 2008 at 10:25 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:

> Sorry, typo. See correction below.
>
> On Wed, Sep 10, 2008 at 10:24 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:
>
>> I propose instead of allow  to have a "values" attribute as
>> a mutually-exclusive alternative to the "value" attribute:
>>
>
> I propose instead to allow  to have a "values" attribute...
> (ditto above)
>
>

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

[gwt-contrib] Re: Code Review Request - Issue 2869 - Add gwt-TabBarFirst-wrapper and gwt-TabBarRest-wrapper styles

[Emily Crutcher]

LGTM

On Wed, Sep 10, 2008 at 11:24 AM, John LaBanca <[EMAIL PROTECTED]> wrote:

> Emily -
>
> Please do a code review on this patch which adds some additional styles to
> TabBar.
>
> Description:
> =
> It would be nice to add the styles gwt-TabBarFirstWrapper and
> gwt-TabBarRestWrapper to the TD cells that wrap the gwt-TabBarFirst and
> gwt-TabBarRest divs so the cells themselves can be styled. Currently, we do
> provide the style gwt-TabBarItemWrapper on the TD cells around TabBar
> items.  The primary use case is to hide the td cell that contains the
> TabBarRest element so it doesn't push out on its container, which can happen
> in some layouts.
>
> Fix:
> ===
> Added the styles.  I use "gwt-TabBarFirst-wrapper" and
> "gwt-TabBarRest-wrapper" to be consistent with the existing style
> "gwt-TabBarItem-wrapper".
>
> Testing:
> ==
> I manually verified that the styles are applied correctly in all main
> browsers.
>
> Thanks,
> John LaBanca
> [EMAIL PROTECTED]
>



-- 
"There are only 10 types of people in the world: Those who understand
binary, and those who don't"

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

[gwt-contrib] Re: new JSON api: Design problem.

[Scott Blum]

On Wed, Sep 10, 2008 at 7:10 AM, Reinier Zwitserloot <[EMAIL PROTECTED]>wrote:

> Here's an alternative idea: Let the new API exist only for reading - I
> never really had an issue with the existing API's utility when
> creating JSON - just when reading it. Solves all these problems. If
> that's fine, then it's ready to go right now, pending an addition to
> the regexp that finds string literals during the validation step as
> per Scott Blum's comments.
>

By the way, that was merely what I thought of off the top of my head.  I
kinda feel like this proposal should circulate to a much broader audience
even than GWTC, where some experts in secure JSON parsing could weigh in,
before I'd be comfortable advertising it as a "safe" parse.

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

[gwt-contrib] Code Review Request - Issue 2869 - Add gwt-TabBarFirst-wrapper and gwt-TabBarRest-wrapper styles

[John LaBanca]

Emily -

Please do a code review on this patch which adds some additional styles to
TabBar.

Description:
=
It would be nice to add the styles gwt-TabBarFirstWrapper and
gwt-TabBarRestWrapper to the TD cells that wrap the gwt-TabBarFirst and
gwt-TabBarRest divs so the cells themselves can be styled. Currently, we do
provide the style gwt-TabBarItemWrapper on the TD cells around TabBar
items.  The primary use case is to hide the td cell that contains the
TabBarRest element so it doesn't push out on its container, which can happen
in some layouts.

Fix:
===
Added the styles.  I use "gwt-TabBarFirst-wrapper" and
"gwt-TabBarRest-wrapper" to be consistent with the existing style
"gwt-TabBarItem-wrapper".

Testing:
==
I manually verified that the styles are applied correctly in all main
browsers.

Thanks,
John LaBanca
[EMAIL PROTECTED]

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

Index: user/src/com/google/gwt/user/client/ui/TabBar.java
===
--- user/src/com/google/gwt/user/client/ui/TabBar.java	(revision 3641)
+++ user/src/com/google/gwt/user/client/ui/TabBar.java	(working copy)
@@ -28,7 +28,11 @@
  * 
  * .gwt-TabBar { the tab bar itself }
  * .gwt-TabBar .gwt-TabBarFirst { the left edge of the bar }
+ * .gwt-TabBar .gwt-TabBarFirst-wrapper { table cell around the left edge }
+ * 
  * .gwt-TabBar .gwt-TabBarRest { the right edge of the bar }
+ * .gwt-TabBar .gwt-TabBarRest-wrapper { table cell around the right edge }
+ * 
  * .gwt-TabBar .gwt-TabBarItem { unselected tabs }
  * .gwt-TabBar .gwt-TabBarItem-wrapper { table cell around tab }
  * .gwt-TabBar .gwt-TabBarItem-selected { additional style for selected
@@ -122,6 +126,10 @@
 first.setHeight("100%");
 panel.setCellHeight(first, "100%");
 panel.setCellWidth(rest, "100%");
+setStyleName(first.getElement().getParentElement(),
+"gwt-TabBarFirst-wrapper");
+setStyleName(rest.getElement().getParentElement(),
+"gwt-TabBarRest-wrapper");
   }
 
   /**

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Bruce Johnson]

Sorry, typo. See correction below.

On Wed, Sep 10, 2008 at 10:24 AM, Bruce Johnson <[EMAIL PROTECTED]> wrote:

> I propose instead of allow  to have a "values" attribute as a
> mutually-exclusive alternative to the "value" attribute:
>

I propose instead to allow  to have a "values" attribute...
(ditto above)

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Bruce Johnson]

One quick pitch for reconsidering the choice of overloading the purpose of
the "define-property" tag.
I propose instead of allow  to have a "values" attribute as a
mutually-exclusive alternative to the "value" attribute:

  

The purpose of the  tag is to serve as the authoritative
definition of a property. As a result, it is always possible to definitively
answer the question, "Which module defined property X?" This is useful
clarity; I think  also has a lot of clarity, which seems
like a positive. You really aren't intending to *define* a property with
this construct, you are intending to *restrict* a property.

We currently have:





Of the three, this functionality seems much more similar to 
than .

-- Bruce

On Wed, Sep 10, 2008 at 9:40 AM, John Tamplin <[EMAIL PROTECTED]> wrote:

> On Tue, Sep 9, 2008 at 7:33 PM, BobV <[EMAIL PROTECTED]> wrote:
>
>> This patch against trunk allows multiple define-property tags for the
>> same deferred binding property.  The second and subsequent definitions
>> of a property will override the allowable values and undo the effects
>> of any previous set-value tags.  No change will be made to the
>> existing property-provider.
>>
>> For example, to build just a module for Safari and IE6, you could write:
>>  
>>
>> Diffstat:
>>  ModuleDefSchema.java |   179 + 8 - 0 !
>>  Property.java|   2517 +8 - 0 !
>>  2 files changed, 26 insertions(+), 16 deletions(-)
>>
>
> LGTM.  Should there be a test?
>
> --
> John A. Tamplin
> Software Engineer (GWT), Google
>
> >
>

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

[gwt-contrib] Re: [gwt-google-apis commit] r771 - trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client

[Miguel Méndez]

LGTM

On Wed, Sep 10, 2008 at 10:20 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hi Miguel,
>
> I left some files from the HelloMaps sample out of the patch from the last
> review.  Could you TBR them for me?
>
>
> -- Forwarded message --
> From: <[EMAIL PROTECTED]>
> Date: Wed, Sep 10, 2008 at 10:15 AM
> Subject: [gwt-google-apis commit] r771 -
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client
> To: [EMAIL PROTECTED]
>
>
>
> Author: [EMAIL PROTECTED]
> Date: Wed Sep 10 07:14:41 2008
> New Revision: 771
>
> Modified:
>
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
>
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
>
> Log:
> Demos needed updating after Icon conversion to JS overlays.
>
> Patch by: zundel
> Review by: mmendez(TBR)
>
>
>
> Modified:
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
>
> ==
> ---
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
> (original)
> +++
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
> Wed Sep 10 07:14:41 2008
> @@ -84,10 +84,10 @@
>
>  // Create a base icon for all of our markers that specifies the
>  // shadow, icon dimensions, etc.
> -baseIcon = new Icon();
> +baseIcon = Icon.newInstance();
>  baseIcon.setShadowURL("http://www.google.com/mapfiles/shadow50.png";);
> -baseIcon.setIconSize(new Size(20, 34));
> -baseIcon.setShadowSize(new Size(37, 34));
> +baseIcon.setIconSize(Size.newInstance(20, 34));
> +baseIcon.setShadowSize(Size.newInstance(37, 34));
>  baseIcon.setIconAnchor(Point.newInstance(9, 34));
>  baseIcon.setInfoWindowAnchor(Point.newInstance(9, 2));
>  // TOOD(sgross): undocumented?
> @@ -118,7 +118,7 @@
>private Marker createMarker(LatLng point, int index) {
>  // Create a lettered icon for this point using our icon class
>  final char letter = (char) ('A' + index);
> -Icon icon = new Icon(baseIcon);
> +Icon icon = Icon.newInstance(baseIcon);
>  icon.setImageURL("http://www.google.com/mapfiles/marker"; + letter
> + ".png");
>  MarkerOptions options = MarkerOptions.newInstance();
>  options.setIcon(icon);
>
> Modified:
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
>
> ==
> ---
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
> (original)
> +++
>
> trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
> Wed Sep 10 07:14:41 2008
> @@ -82,11 +82,11 @@
>  map.clearOverlays();
>
>  // Create our "tiny" marker icon
> -Icon icon = new Icon(
> +Icon icon = Icon.newInstance(
>  "http://labs.google.com/ridefinder/images/mm_20_red.png";);
>
> icon.setShadowURL("
> http://labs.google.com/ridefinder/images/mm_20_shadow.png";);
> -icon.setIconSize(new Size(12, 20));
> -icon.setShadowSize(new Size(22, 20));
> +icon.setIconSize(Size.newInstance(12, 20));
> +icon.setShadowSize(Size.newInstance(22, 20));
>  icon.setIconAnchor(Point.newInstance(6, 20));
>  icon.setInfoWindowAnchor(Point.newInstance(5, 1));
>
>
> >
>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Fwd: [gwt-google-apis commit] r771 - trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client

[Eric Ayers]

Hi Miguel,

I left some files from the HelloMaps sample out of the patch from the last
review.  Could you TBR them for me?

-- Forwarded message --
From: <[EMAIL PROTECTED]>
Date: Wed, Sep 10, 2008 at 10:15 AM
Subject: [gwt-google-apis commit] r771 -
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client
To: [EMAIL PROTECTED]



Author: [EMAIL PROTECTED]
Date: Wed Sep 10 07:14:41 2008
New Revision: 771

Modified:

trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java

trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java

Log:
Demos needed updating after Icon conversion to JS overlays.

Patch by: zundel
Review by: mmendez(TBR)



Modified:
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
==
---
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
(original)
+++
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconClassDemo.java
Wed Sep 10 07:14:41 2008
@@ -84,10 +84,10 @@

 // Create a base icon for all of our markers that specifies the
 // shadow, icon dimensions, etc.
-baseIcon = new Icon();
+baseIcon = Icon.newInstance();
 baseIcon.setShadowURL("http://www.google.com/mapfiles/shadow50.png";);
-baseIcon.setIconSize(new Size(20, 34));
-baseIcon.setShadowSize(new Size(37, 34));
+baseIcon.setIconSize(Size.newInstance(20, 34));
+baseIcon.setShadowSize(Size.newInstance(37, 34));
 baseIcon.setIconAnchor(Point.newInstance(9, 34));
 baseIcon.setInfoWindowAnchor(Point.newInstance(9, 2));
 // TOOD(sgross): undocumented?
@@ -118,7 +118,7 @@
   private Marker createMarker(LatLng point, int index) {
 // Create a lettered icon for this point using our icon class
 final char letter = (char) ('A' + index);
-Icon icon = new Icon(baseIcon);
+Icon icon = Icon.newInstance(baseIcon);
 icon.setImageURL("http://www.google.com/mapfiles/marker"; + letter
+ ".png");
 MarkerOptions options = MarkerOptions.newInstance();
 options.setIcon(icon);

Modified:
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
==
---
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
(original)
+++
trunk/maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/IconDemo.java
Wed Sep 10 07:14:41 2008
@@ -82,11 +82,11 @@
 map.clearOverlays();

 // Create our "tiny" marker icon
-Icon icon = new Icon(
+Icon icon = Icon.newInstance(
 "http://labs.google.com/ridefinder/images/mm_20_red.png";);

icon.setShadowURL("http://labs.google.com/ridefinder/images/mm_20_shadow.png
");
-icon.setIconSize(new Size(12, 20));
-icon.setShadowSize(new Size(22, 20));
+icon.setIconSize(Size.newInstance(12, 20));
+icon.setShadowSize(Size.newInstance(22, 20));
 icon.setIconAnchor(Point.newInstance(6, 20));
 icon.setInfoWindowAnchor(Point.newInstance(5, 1));






-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Size and Icon to JS overlays

[Miguel Méndez]

On Wed, Sep 10, 2008 at 9:53 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> On Wed, Sep 10, 2008 at 9:32 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:
>
>> On Tue, Sep 9, 2008 at 11:17 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> Hello Miguel,
>>>
>>> I would like for you to review the attached patch.
>>>
>>> This change converts the Size and Icon classes to JavaScript overlays,
>>> meaning you must use the 'newInstance()' static method to construct them.  I
>>> did them together because there were inter-dependencies.
>>>
>>> I also broke up the Marker test and Icon test into two separate test
>>> cases.  The getImageMap() and setImageMap() now have a JsArray version and
>>> an int[] version.
>>>
>>
>>> M
>>> maps/maps/src/com/google/gwt/maps/client/control/ControlPosition.java
>>>
>> LG - We do plan on converting ControlPosition as well right?
>>
> We can, but it is only used once or twice per map, so I didn't tag it as
> high priority.
>

Ok.


>>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>>>
>> 54 - typo "creae" -> "create"
>>
> Fixed.
>
Ok.


>
>
>>
>>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Icon.java
>>>
>> LG with one nit: 41 - why do we set the anchor to 0,0 for newInstance()?
>>
>
> The other constructor had a comment which I copied here:
>
> // Workaround for problem in the Maps API - issue 124
>
> Issue 124 has to do with exceptions from the JS API if the anchor is not
> set.
>
Ok.

LGTM

-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Size and Icon to JS overlays

[Eric Ayers]

On Wed, Sep 10, 2008 at 10:01 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

>
> On Wed, Sep 10, 2008 at 9:53 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> On Wed, Sep 10, 2008 at 9:32 AM, Miguel Méndez <[EMAIL PROTECTED]>wrote:
>>
>>> On Tue, Sep 9, 2008 at 11:17 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
 Hello Miguel,

 I would like for you to review the attached patch.

 This change converts the Size and Icon classes to JavaScript overlays,
 meaning you must use the 'newInstance()' static method to construct them.  
 I
 did them together because there were inter-dependencies.

 I also broke up the Marker test and Icon test into two separate test
 cases.  The getImageMap() and setImageMap() now have a JsArray version and
 an int[] version.

>>>
 M
 maps/maps/src/com/google/gwt/maps/client/control/ControlPosition.java

>>> LG - We do plan on converting ControlPosition as well right?
>>>
>> We can, but it is only used once or twice per map, so I didn't tag it as
>> high priority.
>>
>
> Ok.
>
>
 M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java

>>> 54 - typo "creae" -> "create"
>>>
>> Fixed.
>>
> Ok.
>
>
>>
>>
>>>
 M  maps/maps/src/com/google/gwt/maps/client/overlay/Icon.java

>>> LG with one nit: 41 - why do we set the anchor to 0,0 for newInstance()?
>>>
>>
>> The other constructor had a comment which I copied here:
>>
>> // Workaround for problem in the Maps API - issue 124
>>
>> Issue 124 has to do with exceptions from the JS API if the anchor is not
>> set.
>>
> Ok.
>
> LGTM
>

Thanks for the review.  Committed as r770.



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: gwt-google-apis gears package.html updates

[Miguel Méndez]

LGTM

On Tue, Sep 9, 2008 at 4:43 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hi Miguel,
>
> I would like for you to review this addition of package level javadoc
> summary files for Gears.
>
> A  gears/src/com/google/gwt/gears/offline/client/package.html
> A  gears/src/com/google/gwt/gears/client/localserver/package.html
> A  gears/src/com/google/gwt/gears/client/workerpool/package.html
> A  gears/src/com/google/gwt/gears/client/database/package.html
> A  gears/src/com/google/gwt/gears/client/package.html
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Bounds to JavaScript overlay

[Miguel Méndez]

On Wed, Sep 10, 2008 at 9:43 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> On Wed, Sep 10, 2008 at 9:13 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:
>
>> On Mon, Sep 8, 2008 at 5:37 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>
>>> M  maps/maps/src/com/google/gwt/maps/client/geom/Bounds.java
>>>
>> 37 - This Bounds construction is not part of the Maps API proper and it is
>> only called from our test code.  Do we still need it?
>>
> I added this as a convenience, because I was tired of constructing JsArrays
> over and over in the test code.  It results in compiled code just as tight
> or tighter (if you don't already have Point objects made).  We could add a
> 3rd constructor with 2 point objects.
>

Sounds like it should be in the test code then.  If anything you could
create a variadic method to take Point arguments and return a
JsArray.  I would not worry about a third constructor for now.


>
>
>
>> 105, 114, 123, 132 - Should get[Max|Min][X|Y] just return doubles?  The JS
>> API just states that this is a Number however the documentation would lead
>> you to believe that it is really an integer value.  It seems like the safest
>> thing to do would be to use a double.  Maybe Pam could shed some light?
>>  This would call into question the newPoint method.
>>
>
> This sounds familiar - have we been through this before?  Along with Point,
> this object represents pixels on the screen.  Of course, JS doesn't have an
> integer type, but I see in the JS code that pixel values (stored in the
> Bounds object) are looped through with for loops with operators like x++ and
> y--, indicating to me they are intended to be integers.
>
>

We have been through this before, but it seems that every instance in the
maps API is unique.  ++ and -- are valid for floating point also it might be
more illuminating to look at the terminating condition for the loop.  Better
yet, just double check with Pam.

-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Size and Icon to JS overlays

[Eric Ayers]

On Wed, Sep 10, 2008 at 9:32 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

>
>
> On Tue, Sep 9, 2008 at 11:17 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello Miguel,
>>
>> I would like for you to review the attached patch.
>>
>> This change converts the Size and Icon classes to JavaScript overlays,
>> meaning you must use the 'newInstance()' static method to construct them.  I
>> did them together because there were inter-dependencies.
>>
>> I also broke up the Marker test and Icon test into two separate test
>> cases.  The getImageMap() and setImageMap() now have a JsArray version and
>> an int[] version.
>>
>> M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
>>
> LG
>
>
>> M  maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java
>>
> LG
>
>
>> A  maps/maps/test/com/google/gwt/maps/client/overlay/IconTest.java
>>
> LG
>
>
>>
>> M
>> maps/maps/src/com/google/gwt/maps/client/control/ControlPosition.java
>>
> LG - We do plan on converting ControlPosition as well right?
>
We can, but it is only used once or twice per map, so I didn't tag it as
high priority.


>
>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>>
> 54 - typo "creae" -> "create"
>
Fixed.


>
>> M  maps/maps/src/com/google/gwt/maps/client/overlay/Icon.java
>>
> LG with one nit: 41 - why do we set the anchor to 0,0 for newInstance()?
>

The other constructor had a comment which I copied here:

// Workaround for problem in the Maps API - issue 124

Issue 124 has to do with exceptions from the JS API if the anchor is not
set.


>
>
>>
>> M  maps/maps/src/com/google/gwt/maps/client/overlay/MarkerOptions.java
>>
> LG
>
>>
>> M  maps/maps/src/com/google/gwt/maps/client/geom/Size.java
>>
> LG
>
>
>>  M  maps/maps/src/com/google/gwt/maps/client/MapWidget.java
>>
>
>
>>
>> D  maps/maps/src/com/google/gwt/maps/client/impl/SizeImpl.java
>>
> LG
>
>
>> D  maps/maps/src/com/google/gwt/maps/client/impl/IconImpl.java
>>
> LG
>
>
>>
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
>
> --
> Miguel
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps convert PolyXXXOptions to JavaScript overlays

[Miguel Méndez]

On Tue, Sep 9, 2008 at 3:54 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hello Miguel,
>
> I've combined the conversion of several classes into one patch because of
> the overlap.
>
>  - PolylineOptions
>  - PolygonOptions
>  - Fixed constructor name in PolyStyleOptions.java
>  - Fixed references to above.
>  - Changed some references to PolylineOptions to EncodedPolyline... that
> was weird!
>
> M  maps/maps/test/com/google/gwt/maps/client/overlay/PolygonTest.java
>
LG


>
> M  maps/maps/src/com/google/gwt/maps/client/overlay/PolygonOptions.java
>
LG - consider adding a newInstance(boolean clickable) method.


>
> A
> maps/maps/src/com/google/gwt/maps/client/overlay/PolylineOptions.java
>
LG - This is one place where we could provide an additional newInstance
overload to specify clickable and geodesic.


>
> M
> maps/maps/src/com/google/gwt/maps/client/overlay/EncodedPolyline.java
>
LG


>
> M
> maps/maps/src/com/google/gwt/maps/client/overlay/PolyStyleOptions.java
>
LG


> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>
LG


> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polyline.java
>
LG


> D
> maps/maps/src/com/google/gwt/maps/client/impl/PolyStyleOptionsImpl.java
>
LG


> M
> maps/maps/src/com/google/gwt/maps/client/impl/PolylineFactoryImpl.java
>
LG - Is this an overlay conversion candidate?

D
> maps/maps/src/com/google/gwt/maps/client/impl/PolygonOptionsImpl.java
>
LG


> D
> maps/maps/src/com/google/gwt/maps/client/impl/PolylineOptionsImpl.java
>
LG


> M
> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/DrawingOverlayDemo.java
>
LG

M
> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/OverlayDemo.java
>
LG


>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Bounds to JavaScript overlay

[Eric Ayers]

On Wed, Sep 10, 2008 at 9:13 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

> On Mon, Sep 8, 2008 at 5:37 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hello Miguel,
>>
>> This is the halfway point - 9 classes down, 9 to go.
>>
>> I would like for you to review the attached patch which converts the
>> Bounds object to a JavaScript overlay.  I changed the behavior of the
>> 'extends()' method to just mirror the JavaScript method instead of cloning
>> the bounds object.  I also added a new factory method which takes 4 int
>> arguments.
>>
>> Since I made those changes and there were no references to this object I
>> could test with, I added a unit test.
>>
>> A  maps/maps/test/com/google/gwt/maps/client/geom/BoundsTest.java
>>
> LG
>
>
>>
>> M  maps/maps/src/com/google/gwt/maps/client/geom/Bounds.java
>>
> 37 - This Bounds construction is not part of the Maps API proper and it is
> only called from our test code.  Do we still need it?
>
I added this as a convenience, because I was tired of constructing JsArrays
over and over in the test code.  It results in compiled code just as tight
or tighter (if you don't already have Point objects made).  We could add a
3rd constructor with 2 point objects.



> 105, 114, 123, 132 - Should get[Max|Min][X|Y] just return doubles?  The JS
> API just states that this is a Number however the documentation would lead
> you to believe that it is really an integer value.  It seems like the safest
> thing to do would be to use a double.  Maybe Pam could shed some light?
>  This would call into question the newPoint method.
>

This sounds familiar - have we been through this before?  Along with Point,
this object represents pixels on the screen.  Of course, JS doesn't have an
integer type, but I see in the JS code that pixel values (stored in the
Bounds object) are looped through with for loops with operators like x++ and
y--, indicating to me they are intended to be integers.


>
>> D  maps/maps/src/com/google/gwt/maps/client/impl/BoundsImpl.java
>>
> LG
>
>>
>> M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
>>
> LG
>
>>
>> -Eric.
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
>
> --
> Miguel
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[John Tamplin]

On Tue, Sep 9, 2008 at 7:33 PM, BobV <[EMAIL PROTECTED]> wrote:

> This patch against trunk allows multiple define-property tags for the
> same deferred binding property.  The second and subsequent definitions
> of a property will override the allowable values and undo the effects
> of any previous set-value tags.  No change will be made to the
> existing property-provider.
>
> For example, to build just a module for Safari and IE6, you could write:
>  
>
> Diffstat:
>  ModuleDefSchema.java |   179 + 8 - 0 !
>  Property.java|   2517 +8 - 0 !
>  2 files changed, 26 insertions(+), 16 deletions(-)
>

LGTM.  Should there be a test?

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

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

[gwt-contrib] Re: RR : Allow to redefine an existing property

[Thomas Broyer]

On 10 sep, 04:38, "Fred Sauer" <[EMAIL PROTECTED]> wrote:
> +1
> I think this would be very useful.

+1 too; moreover given than  already supports such
overrides...


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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Size and Icon to JS overlays

[Miguel Méndez]

On Tue, Sep 9, 2008 at 11:17 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hello Miguel,
>
> I would like for you to review the attached patch.
>
> This change converts the Size and Icon classes to JavaScript overlays,
> meaning you must use the 'newInstance()' static method to construct them.  I
> did them together because there were inter-dependencies.
>
> I also broke up the Marker test and Icon test into two separate test
> cases.  The getImageMap() and setImageMap() now have a JsArray version and
> an int[] version.
>
> M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
>
LG


> M  maps/maps/test/com/google/gwt/maps/client/overlay/MarkerTest.java
>
LG


> A  maps/maps/test/com/google/gwt/maps/client/overlay/IconTest.java
>
LG


>
> M
> maps/maps/src/com/google/gwt/maps/client/control/ControlPosition.java
>
LG - We do plan on converting ControlPosition as well right?

>
> M  maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>
54 - typo "creae" -> "create"

>
> M  maps/maps/src/com/google/gwt/maps/client/overlay/Icon.java
>
LG with one nit: 41 - why do we set the anchor to 0,0 for newInstance()?


>
> M  maps/maps/src/com/google/gwt/maps/client/overlay/MarkerOptions.java
>
LG

>
> M  maps/maps/src/com/google/gwt/maps/client/geom/Size.java
>
LG


> M  maps/maps/src/com/google/gwt/maps/client/MapWidget.java
>


>
> D  maps/maps/src/com/google/gwt/maps/client/impl/SizeImpl.java
>
LG


> D  maps/maps/src/com/google/gwt/maps/client/impl/IconImpl.java
>
LG


>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Route conversion to JS overlay

[Eric Ayers]

On Wed, Sep 10, 2008 at 8:56 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

> LGTM - nits: 40, 49 won't work until Duration and Distance are converted.
>  Maybe you should add a TODO like you did on line 74 until those are
> converted?
>

Good catch. Duration and Distance weren't on my list of classes to convert.
Committed as r769.


>
>
> On Mon, Sep 8, 2008 at 3:51 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> Hi Miguel,
>>
>> I would like for you to review the attached patch.  It converts the Route
>> object to a JavaScript overlay.
>>
>> D  maps/maps/src/com/google/gwt/maps/client/impl/RouteImpl.java
>> M  maps/maps/src/com/google/gwt/maps/client/geocode/Route.java
>> --
>> Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> http://code.google.com/webtoolkit/
>>
>
>
>
> --
> Miguel
>



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Convert Bounds to JavaScript overlay

[Miguel Méndez]

On Mon, Sep 8, 2008 at 5:37 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hello Miguel,
>
> This is the halfway point - 9 classes down, 9 to go.
>
> I would like for you to review the attached patch which converts the Bounds
> object to a JavaScript overlay.  I changed the behavior of the 'extends()'
> method to just mirror the JavaScript method instead of cloning the bounds
> object.  I also added a new factory method which takes 4 int arguments.
>
> Since I made those changes and there were no references to this object I
> could test with, I added a unit test.
>
> A  maps/maps/test/com/google/gwt/maps/client/geom/BoundsTest.java
>
LG


>
> M  maps/maps/src/com/google/gwt/maps/client/geom/Bounds.java
>
37 - This Bounds construction is not part of the Maps API proper and it is
only called from our test code.  Do we still need it?
105, 114, 123, 132 - Should get[Max|Min][X|Y] just return doubles?  The JS
API just states that this is a Number however the documentation would lead
you to believe that it is really an integer value.  It seems like the safest
thing to do would be to use a double.  Maybe Pam could shed some light?
 This would call into question the newPoint method.


> D  maps/maps/src/com/google/gwt/maps/client/impl/BoundsImpl.java
>
LG

>
> M  maps/maps/test/com/google/gwt/maps/MapsTestSuite.java
>
LG

>
> -Eric.
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] Re: RR : Three new RPC design docs

[John Tamplin]

On Wed, Sep 10, 2008 at 3:52 AM, Johan Rydberg <[EMAIL PROTECTED]>wrote:

> I just say that I would like to see a format that doesn't make it harder
> for non-Java backend implementations, than it already is.
>

I think non-Java servers can best be accomodated by a separate protobuffer
RPC implementation, rather than trying to neuter the Java one to accomodate
it.

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

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

[gwt-contrib] Re: Code Review: gwt-google-apis Maps Route conversion to JS overlay

[Miguel Méndez]

LGTM - nits: 40, 49 won't work until Duration and Distance are converted.
 Maybe you should add a TODO like you did on line 74 until those are
converted?

On Mon, Sep 8, 2008 at 3:51 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:

> Hi Miguel,
>
> I would like for you to review the attached patch.  It converts the Route
> object to a JavaScript overlay.
>
> D  maps/maps/src/com/google/gwt/maps/client/impl/RouteImpl.java
> M  maps/maps/src/com/google/gwt/maps/client/geocode/Route.java
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>



-- 
Miguel

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

[gwt-contrib] new JSON api: Design problem.

[Reinier Zwitserloot]

I didn't quite think the way you can use the new API to create new
JSON objects through. Specifically, in order to support the 'set'
notation, Any given lookup will always need to tree out from the root,
every time, which could be a little slow for very deeply nested JSON,
and take up more memory.

In other words:

JSON json = JSON.parse(/*deeply nested
json*/).get("foo").get("bar").get(10).get("baz");

would have to be just the original deeply nested json and a 'pointer'
to foo/bar/10/baz. So, it will still take up all the memory of the big
json object, and every access to this thing will result internally in:

jso["foo"]["bar"][10]["baz"].

I also have some doubts that the writing part of the API will make
sense to the casual user. It's definitely fast and convenient if you
know how it works, but, does this look readable to you (objective:
create {"foo": [10, null], "bar": []}:


JSON json = JSON.createMap();
json.get("foo").add().set(10);
json.get("foo").add().setNull();
json.get("bar").set(JSON.createList());

(one of the problems is that its really difficult to create empty
lists/maps with this method, as the general idea is to create the
'structure' (JS arrays and objects) on demand, but there is no demand
if they are supposed to be empty!)

Contrast this to the IMHO much cleaner code you can use to read such a
structure:

json.get("foo").get(0).asInt();
json.get("foo").get(1).isNull();
for (json item : JSON.get("foo").asList()) {
  Object o = item.asObject();
}
json.get("bar").asList().size();
json.asObject(); //returns a HashMap with 2 ArrayLists in it, one of
which has an Integer and 'null', the other is empty).


Here's an alternative idea: Let the new API exist only for reading - I
never really had an issue with the existing API's utility when
creating JSON - just when reading it. Solves all these problems. If
that's fine, then it's ready to go right now, pending an addition to
the regexp that finds string literals during the validation step as
per Scott Blum's comments.


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

[gwt-contrib] Re: New JSON API proposal

[Reinier Zwitserloot]

I can filter out newlines in string literals to translate to invalid
JSON, but alternative non-standard methods to terminate string
literals - there's no way I can safeguard against this.

On Sep 8, 5:07 pm, "Scott Blum" <[EMAIL PROTECTED]> wrote:
> I think string literals need special scrutiny.  If there is any way to get
> the parser to break out of a string literal where your checker doesn't
> notice, it would be a big problem  For example, if there are parsers that
> will allow a carriage return to terminate a string literal and continue
> evaluating code, your checker could miss it.  Or if there are alternate ways
> to end a string literal, like somehow encoding a character that will be
> recognized as an end quote.
> On Sun, Sep 7, 2008 at 6:38 PM, Reinier Zwitserloot <[EMAIL PROTECTED]>wrote:
>
> > I haven't tested it yet, but I'm throwing it out there for review of
> > the concept of what's going on here. Obviously, after this method is
> > done checking the json string, it will be eval()ed. Is this deemed
> > safe enough?
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---

[gwt-contrib] Re: RR : Three new RPC design docs

[Johan Rydberg]

> This document is more about how we might revamp the RPC payload format
> to improve deserialization speed, eliminate recursive behavior, and
> provide a well-supported mechanism for custom serialization:
>   http://code.google.com/p/google-web-toolkit/wiki/RpcDirectEval

I just say that I would like to see a format that doesn't make it harder
for non-Java backend implementations, than it already is.

I can think of these right now:

   http://code.google.com/p/python-gwt-rpc/
   http://code.google.com/p/gwtphp/
   http://code.google.com/p/twisted-gwt/  (my project)

(Not suggesting that the proposed format is anyway harder to encode to).

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