Change in gwt[master]: Fixes ISSUE 7079 - Add support for the newer bindery Handler...

[Daniel Kurka]

Daniel Kurka has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 1:

Hi Julien,

I can't find you on the CLA-SIGNERS list:
https://code.google.com/p/google-web-toolkit/source/browse/CLA-SIGNERS

Did you already submit a CLA?

-Daniel

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-HasComments: No

[Julien Dramaix]

Julien Dramaix has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 1:

I signed the following one :
https://developers.google.com/open-source/cla/individual?hl=fr

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>

Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>

[Daniel Kurka]

Daniel Kurka has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 1:

That was just me looking at the wrong place for CLAs sorry about that!

I am still learning my around things internally at google.

[Matthew Dempsky]

Matthew Dempsky has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................

Patch Set 1: Verified+1

Hoorays, this change passed presubmit. :D More details at

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>

Gerrit-Reviewer: Matthew Dempsky <mdem...@gwtproject.org>
Gerrit-HasComments: No

[Julien Dramaix]

Hello Matthew Dempsky,

I'd like you to reexamine a change. Please visit

https://gwt-review.googlesource.com/1350

to look at the new patch set (#2).

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................

Fixes ISSUE 7079 - Add support for the newer bindery HandlerRegistration

Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
---
M user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
M user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
2 files changed, 4 insertions(+), 3 deletions(-)

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 2

[Thomas Broyer]

Thomas Broyer has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................

Patch Set 2: Code-Review+1

Hmm, so, UiBinder only supports event handlers that extend EventHandler,
and the error message in this case points to GwtEvent (so we can say
UiBinder is tailored for GwtEvent and not the more generic Event), but this
change wants to make it legal for an addXxxHandler(SomeGwtEvent) method to
return a bindery HandlerRegistration (i.e. mixing c.g.g.event and
c.g.web.bindery.event types in the same method).

That sounds weird.

I wonder if it wouldn't be better to either:

* put an error message when we detect
c.g.web.bindery.event.shared.HandlerRegistration saying "did you mean
com.google.gwt.event.shared.HandlerRegistration"
* or fully support c.g.web.bindery.event: mandate "assignable to
c.g.w.b.event.shared.Event", don't look for EventHandler (only "is
interface"), and thus accept any HandlerRegistration.

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>

Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>

Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@gwtproject.org>

Gerrit-Reviewer: Thomas Broyer <t.br...@gmail.com>
Gerrit-HasComments: No

[Manuel Carrasco Moñino]

Manuel Carrasco Moñino has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 2: Code-Review+1

@Julien LGTM

@Thomas, I think #2 could be a good option

[Julien Dramaix]

Julien Dramaix has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 2:

@Thomas I agree with your option #2 but we should create an issue for that
and fix it in another change.
The goal of the issue 7079 is to support new HandlerRegistration class

What do you think ?

[Goktug Gokdogan]

Goktug Gokdogan has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................

Patch Set 2: Code-Review+1

It is weird but I don't think it is a big deal as it is technically correct
to return the super type of HandlerRegistration from your addXXXHandler
method.

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>

Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>

[Matthew Dempsky]

Matthew Dempsky has posted comments on this change.

Change subject: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
......................................................................


Patch Set 2:

(1 comment)

....................................................
Commit Message
Line 7: Fixes ISSUE 7079 - Add support for the newer bindery
HandlerRegistration
I think the commit message could be a little more descriptive.

Also, could you please change it to something like:

Add support for the newer bindery HandlerRegistration

Bug: issue 7079
Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

Then Gerrit can autolink to issue 7079.

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 2
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>

Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>

Gerrit-Reviewer: Matthew Dempsky <mdem...@gwtproject.org>
Gerrit-Reviewer: Thomas Broyer <t.br...@gmail.com>

Gerrit-HasComments: Yes

[Thomas Broyer]

Thomas Broyer has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

....................................................
Commit Message

Line 9: Bug: issue 7079
Ah, footer lines in Git must come as a single block/paragraph with no blank
lines ;-)

Bug: issue 7079
Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

Gerrit-PatchSet: 3

[Julien Dramaix]

Julien Dramaix has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration

Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................


Patch Set 4:

Why the build failed ?

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

Gerrit-PatchSet: 4

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@gwtproject.org>
Gerrit-Reviewer: Thomas Broyer <t.br...@gmail.com>

Gerrit-HasComments: No

[Roberto Lublinerman]

Roberto Lublinerman has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................


Patch Set 4:

The Jenkins server run out of disk (I think) and this failure might be the
result of that. It has been fixed now but I don't think this patch will be
resubmitted for testing.

For sure when a new patch is uploaded, it will be tested.

To see the output just click on the Jenkins link and and then go to console
output.

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 4
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@gwtproject.org>

Gerrit-Reviewer: Roberto Lublinerman <rlu...@google.com>

[Matthew Dempsky]

Hello Manuel Carrasco Moñino, Matthew Dempsky, Thomas Broyer, Leeroy
Jenkins, Goktug Gokdogan,

I'd like you to reexamine a rebased change. Please visit

https://gwt-review.googlesource.com/1350

to look at the new rebased patch set (#5).

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................

Add support for the newer bindery HandlerRegistration

Bug: issue 7079
Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

---
M user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
M user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
2 files changed, 4 insertions(+), 3 deletions(-)

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 5

[Manuel Carrasco Moñino]

Manuel Carrasco Moñino has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................

Patch Set 5: Code-Review+1

LGTM

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 5
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>

Gerrit-Reviewer: Roberto Lublinerman <rlu...@google.com>
Gerrit-Reviewer: Thomas Broyer <t.br...@gmail.com>

Gerrit-HasComments: No

[Thomas Broyer]

Thomas Broyer has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................

Patch Set 5: Code-Review+2

Julien: would you mind creating the issue about fully supporting
com.google.web.bindery.event in UiBinder?

[Thomas Broyer]

Thomas Broyer has submitted this change and it was merged.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................


Add support for the newer bindery HandlerRegistration

Bug: issue 7079
Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850

---
M user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
M user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
2 files changed, 4 insertions(+), 3 deletions(-)

Approvals:
Manuel Carrasco Moñino: Looks good to me, but someone else must approve
Leeroy Jenkins: Verified
Thomas Broyer: Looks good to me, approved



diff --git a/user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
b/user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
index 91a5c5f..cc7bc2c 100644
--- a/user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
+++ b/user/src/com/google/gwt/uibinder/rebind/HandlerEvaluator.java
@@ -23,9 +23,9 @@
import com.google.gwt.core.ext.typeinfo.JType;
import com.google.gwt.core.ext.typeinfo.TypeOracle;
import com.google.gwt.event.shared.EventHandler;
-import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.uibinder.client.UiHandler;
import com.google.gwt.uibinder.rebind.model.OwnerClass;
+import com.google.web.bindery.event.shared.HandlerRegistration;

/**
* This class implements an easy way to bind widget event handlers to
methods
@@ -272,7 +272,8 @@
for (JMethod method : objectType.getInheritableMethods()) {

// Condition 1: returns HandlerRegistration?
- if (method.getReturnType() == handlerRegistrationJClass) {
+ JClassType returnClassType =
method.getReturnType().isClassOrInterface();
+ if (returnClassType != null &&
handlerRegistrationJClass.isAssignableFrom(returnClassType)) {

// Condition 2: single parameter of the same type of handlerType?
JParameter[] parameters = method.getParameters();
diff --git
a/user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
b/user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
index 98de89b..c4a0b1d 100644
--- a/user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
+++ b/user/test/com/google/gwt/uibinder/rebind/HandlerEvaluatorTest.java
@@ -19,8 +19,8 @@
import com.google.gwt.core.ext.typeinfo.TypeOracle;
import com.google.gwt.dev.util.log.PrintWriterTreeLogger;
import com.google.gwt.event.shared.EventHandler;
-import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.uibinder.rebind.model.OwnerClass;
+import com.google.web.bindery.event.shared.HandlerRegistration;

import junit.framework.TestCase;

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: merged

[Julien Dramaix]

Julien Dramaix has posted comments on this change.

Change subject: Add support for the newer bindery HandlerRegistration
Bug: issue 7079 Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
......................................................................

Patch Set 6:

Thomas I created the issue
https://code.google.com/p/google-web-toolkit/issues/detail?id=8182 for the
full support c.g.web.bindery.event in UiBinder.

I will try to propose a patch in few days.

--
To view, visit https://gwt-review.googlesource.com/1350
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80f23b094f55e40d2b2223e9f018c98c4e41a850
Gerrit-PatchSet: 6

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Daniel Kurka <dank...@google.com>
Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Julien Dramaix <julien....@gmail.com>
Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>
Gerrit-Reviewer: Manuel Carrasco Moñino <manuel.c...@gmail.com>
Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>
Gerrit-Reviewer: Roberto Lublinerman <rlu...@google.com>
Gerrit-Reviewer: Thomas Broyer <t.br...@gmail.com>

Gerrit-HasComments: No