Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
yes, you are right, the red squiggle i got in the ide was actually : error: this variable might not have been initialized :| -igor On Fri, Mar 13, 2009 at 1:39 PM, James Carman wrote: > As I said before, my patch for WICKET-2165 doesn't change any test or > example code at all and everything bui

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
As I said before, my patch for WICKET-2165 doesn't change any test or example code at all and everything builds just fine. There are plenty of examples in the test and example code where you pass in a IModel> to the constructor and it all is still working. Take for example the FileListView class

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
I did! I added a simple class to my wicket development project that has all of my changes applied for WICKET-2165. In IDEA when I hit Ctrl-B (go to declaration) it takes me to a constructor with this signature: public ListView(final String id, final IModel> model) If you don't believe me, just

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
now change listview to take imodel> -igor On Fri, Mar 13, 2009 at 1:00 PM, James Carman wrote: > Ok, what part are you talking about that doesn't compile?  In my IDE > right now, I have: > > public class ListManager extends Panel > { >    public ListManager(String id, IModel> entries) >    { >  

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
Ok, what part are you talking about that doesn't compile? In my IDE right now, I have: public class ListManager extends Panel { public ListManager(String id, IModel> entries) { super(id); add(new ListView("list", entries) { protected void populateItem(L

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
yes, sorry class listmanager extends panel { public listmanager(string id, imodel> entries) { add(new form("form") { onsubmit() { listmanager.this.getmodelobject().add(someentry);}} add(new listview("list", entries){}); -igor On Fri, Mar 13, 2009 at 11:51 AM, James Carman wrote: > If T is

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
If T is the element type, then you can't use an IModel for the choices. You can't do that with the current API. Did you mean to pass in an IModel> to the constructor? On Mar 13, 2009 2:45 PM, "Igor Vaynberg" wrote: t is the type of a list element, see below class listmanager extends panel { p

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
t is the type of a list element, see below class listmanager extends panel { public listmanager(string id, imodel entries) { add(new form("form") { onsubmit() { listmanager.this.getmodelobject().add(someentry);}} add(new listview("list", entries){}); ^ wont compile because you cant as

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
T has to be a list of some sort in this case and you would want to convey that in your API. Are you wanting T to be the element type here or the list type itself. I'm really having a hard time following what you're trying to do here. What type parameter are you passing into your listview? On Fr

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
the problem remains. lets say we change listview to take imodel> i want to do something like this: class listmanger extends panel { public listmanager(string id, imodel entries) { addformtoaddtems(); add(new listview("list", entities)); ^ wont compile because you cant assign imod

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
Yes, but that's fine. The idea is that I want to collect a List, but I want to choose from a List. That should be allowed. On Fri, Mar 13, 2009 at 10:42 AM, Igor Vaynberg wrote: > so then again you are back to something taking imodel> and a > widened version in the same constructor. > > -igor

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
so then again you are back to something taking imodel> and a widened version in the same constructor. -igor On Fri, Mar 13, 2009 at 3:54 AM, James Carman wrote: > Again, ListMultipleChoice is okay in this instance.  You don't want to > "widen" the model.  You want to widen what can go in it (the

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
Again, ListMultipleChoice is okay in this instance. You don't want to "widen" the model. You want to widen what can go in it (the choices), which I have. On Fri, Mar 13, 2009 at 3:40 AM, Igor Vaynberg wrote: > taking ListMultipleChoice as another example, your patch leaves it us > > public clas

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread James Carman
Your right, my patch was a bit off with respect to Palette, but not because of this. I shouldn't have "widened" the second constructor. It should take a IModel>. So, the getModelCollection() doesn't have to change. When we're doing this widening, only the "choices" (not the model) should be wide

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
taking ListMultipleChoice as another example, your patch leaves it us public class ListMultipleChoice extends AbstractChoice, T> when in fact it should be properly declared as public class ListMultipleChoice extends AbstractChoice, T> so that getModelObject() properly returns Collection instead

Re: can we get this patch applied to the current snapshot?

2009-03-13 Thread Igor Vaynberg
your patch works because it does not properly refactor Palette#getModelCollection(). you left it us: public Collection getModelCollection() { return (Collection)getDefaultModelObject(); } which is incorrect, it should in fact now return Collection, which will break

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread James Carman
FYI, I have created a new JIRA issue for my proposed API changes: https://issues.apache.org/jira/browse/WICKET-2165 I've attached a patch. I've "fixed" all of the places I could find thus far. If I find anymore, I'll upload a new patch. James On Fri, Mar 13, 2009 at 1:59 AM, James Carman wr

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread James Carman
"Feeding" the list to your component isn't the issue (the constructor will take what you throw at it). It's using the getDefaultModelObject() method and trying to modify that list once you've passed it in that will cause you to have to cast. On Fri, Mar 13, 2009 at 1:54 AM, Brill Pappin wrote: >

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Jeremy Thomerson
after removing the "? extends" it will... // THIS WORKS: List list1 = new ArrayList(); list1.add(new Integer(4)); list1.add(4); // THIS DOES NOT WORK: List list2 = new ArrayList(); list2.add(new Integer(4)); list2.add(4); -- Jeremy Thomerson http://www.wickettraining.com On Fri, Mar 13, 2009

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Brill Pappin
I'm really not sure of all the use-cases... all I know is that I spent some time trying to get the darn thing to compile using a fairly basic wicket pattern. If such a simple thing as feeding a list of items to a component that draws a list using them is going to give me that much trouble (

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread James Carman
I didn't change any of the "guts" of Palette (I did change Recorder, a bit) in my patch. So, I guess the answer is "yes", it will work. All of the unit tests pass. On Fri, Mar 13, 2009 at 1:50 AM, Igor Vaynberg wrote: > palette needs to be able to do: > > getmodelobject().clear(); > getmodelobj

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Igor Vaynberg
palette needs to be able to do: getmodelobject().clear(); getmodelobject().add(item); where getmodelobject should return a collection. will that still work with this refactor? i dont see why components that do this need to cast anything to make it work. -igor On Thu, Mar 12, 2009 at 10:43 PM,

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread James Carman
To be clear, the only potential for breakage is where user code tries to modify the returned "model object" (of type List) without casting it. However, that's a rare usecase (in my opinion) and again it's easily overcome by a simple cast. On Thu, Mar 12, 2009 at 5:10 PM, Igor Vaynberg wrote: > b

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Brill Pappin
Sorry, didn't mean to come off accusing :) That was certainly the vote thread, and my personal preference as well... However in the issue I was assured that the new approach was better and compatible with List (I did not test it myself). I think this is certainly something the developers ar

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Jeremy Thomerson
Right - I didn't read the entire series of comments on the JIRA. I did read one that explained why removing the "? extends" was the best solution, and several that agreed with it. I am basing my comments on the vote thread that was on the user list - a change like this will require a vote (as Igo

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Brill Pappin
This is an attempt to bring the dropdown and listview into sync. personally i don't care which way it goes as long as its simple and works, but I think it pretty important that it be done before the 1.4 release. The discussion was continued in the issue, so it's possible that people were n

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Igor Vaynberg
before applying an api-breaking patch to an rc release we should have a vote on the dev list. -igor On Thu, Mar 12, 2009 at 12:59 PM, Jeremy Thomerson wrote: > Unless I'm seeing double - this patch has two problems: > > 1 - It is the opposite of what was voted on.  The vote was to make > IModel>

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Jeremy Thomerson
Unless I'm seeing double - this patch has two problems: 1 - It is the opposite of what was voted on. The vote was to make IModel> into IModel>. Your patch makes it IModel>. 2 - The patch causes compile errors that were not fixed. I have unassigned myself from the task until those issues are ad

Re: can we get this patch applied to the current snapshot?

2009-03-12 Thread Jeremy Thomerson
I'm reviewing it now. Assuming that it looks fine, and is line with what was proposed by the vote thread earlier, I will apply. The vote passed, so I don't see a reason not to. I'm not sure how many were binding / non-binding, but there were eight for, two against. -- Jeremy Thomerson http://ww

can we get this patch applied to the current snapshot?

2009-03-12 Thread Brill Pappin
https://issues.apache.org/jira/browse/WICKET-2137 - Brill - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org