okay, i've talk to eelco a bit now too and i'm convinced.
+1 to remove both PageLink and IPageLink entirely. Jonathan Locke wrote: > > > right. i agree. that's why it's Page and not Class to detemine identity. > > but Class constructor is the 90% case. i think you're right though that > BookmarkablePageLink handles that case. it auto-disables and stores > less state in the page map, right? > > and if you're going to use something beyond class for identity, or you're > not using bookmarkablepagelinks then you're subclassing link. is that > right? like in a case like this: > > editCell.add(new PageLink("edit", new IPageLink() > { > public Page getPage() > { > return new EditDvdDetails(getModel().getNestedModel()); > } > > public Class getPageIdentity() > { > return EditDvdDetails.class; > } > }) > > i'd have to write that linksTo() method you suggested and the getPage > would > be a slightly longer onClick() method. > > what i like about your way is that it eliminates a whole concept and > probably > a lot of confusion. the page identity concept and the interface for > delayed page > creation go away. the expense is that i have to write bigger onClick and > linksTo > methods. that's easier for newbies to understand though... > > shoot. did you just convince me? i hate losing arguments to you, igor. > ;-) > > if bookmarkablepagelink and link really do provide everything i need to > keep auto-link-disabling easy, i guess we ought to remove PageLink and > IPageLink and rename BookmarkablePageLink to PageLink as you suggest. > but only in the 2.0 tree where people are on the bleeding edge. > > > igor.vaynberg wrote: >> >> you are assuming that identity is only defined by the class. but what if >> pageparameters/models are also involved? >> >> then that code has to go somewhere anyways >> >> -igor >> >> >> On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: >>> >>> >>> >>> let's just take a step back and ask: how can we make this less work? >>> >>> could new AutoDisableLink<T> work or does type erasure wipe out >>> that possibility? >>> >>> i mean ideally we could both have what we want here. >>> >>> >>> Jonathan Locke wrote: >>> > >>> > >>> > you meant page.getClass().equals(MyAccountPage.class) >>> > that's quite a mouthful to repeat. it makes something you >>> > don't care about but which i do all the time hard. just >>> > because it's more minimal doesn't make it better. >>> > >>> > i want more abbreviated support for auto-disabling of links >>> > in the core. extracting a subclass of Link like AutoDisableLink >>> > would be fine. but i don't really want to write the code below >>> > by default and i think other users deserve the benefit of this >>> > behavior by default too. it's how wicket makes link menus trivial >>> > and users should not have to go hunting on the wiki to figure >>> > it out. >>> > >>> > >>> > igor.vaynberg wrote: >>> >> >>> >> add( new link("foo") { >>> >> onclick() { setresponsepage(new MyAccountPage()); } >>> >> boolean linksto(Page page) { return >>> >> page.class.equals(MyAccountPage.class); >>> >> } >>> >> } >>> >> >>> >> simple as that and the bookmarkablepagelink already also does this >>> >> >>> >> -igor >>> >> >>> >> >>> >> On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: >>> >>> >>> >>> >>> >>> >>> >>> i don't think you really listened to me. the part i want is not the >>> >>> delayed >>> >>> creation of the page. you can get that by subclassing link. what i >>> >>> want >>> >>> for myself and all wicket users is the /identity/ part. without >>> that >>> >>> wicket >>> >>> cannot disable self-referential links automatically. this makes >>> menuing >>> >>> with links a hassle. if you can explain how i can do delayed >>> linking >>> >>> but >>> >>> still >>> >>> have wicket deal with disabling links to their containing page >>> >>> automatically, >>> >>> i'm fine with that. if not, i'm very much -1 on it. >>> >>> >>> >>> >>> >>> igor.vaynberg wrote: >>> >>> > >>> >>> > why not let us remove it, and you copy it into your project >>> >>> > >>> >>> > -igor >>> >>> > >>> >>> > >>> >>> > On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: >>> >>> >> >>> >>> >> >>> >>> >> >>> >>> >> i don't want to gang up on martijn here, but that's what i meant. >>> >>> >> i also think we should refactor pagelink with the remaining two >>> >>> >> constructors to: >>> >>> >> >>> >>> >> pageclasslink (the one with the class contructor) >>> >>> >> >>> >>> >> and >>> >>> >> >>> >>> >> delayedpagelink (the one with the ipagelink constructor) >>> >>> >> >>> >>> >> this way there is no class name pagelink (which is dangerous >>> because >>> >>> >> it's such an obvious name). this situation will make it very >>> obvious >>> >>> >> what users ought to do (now Link is the most obvious class) and >>> >>> >> pageclasslink >>> >>> >> and delayedpagelink will both be slightly more efficient since >>> they >>> >>> won't >>> >>> >> share an unused field. then users will have a choice between: >>> >>> >> >>> >>> >> link >>> >>> >> pageclasslink >>> >>> >> bookmarkablepagelink >>> >>> >> delayedpagelink >>> >>> >> >>> >>> >> that seems like a very good situation. >>> >>> >> >>> >>> >> if you're still unhappy, martijn, maybe we could keep the page >>> >>> >> constructor >>> >>> >> in a deprecated class like pagereferencelink. that would help >>> guide >>> >>> >> users >>> >>> >> away from the class, but it would still be there for backwards >>> >>> compat. >>> >>> >> >>> >>> >> jon >>> >>> >> >>> >>> >> >>> >>> >> Eelco Hillenius wrote: >>> >>> >> > >>> >>> >> >> Very convenient to hold a vote when I'm not around... >>> >>> >> > >>> >>> >> > It wasn't concluded yet. And the vote wasn't started because of >>> >>> your >>> >>> >> > absence, but because of a message to the list. >>> >>> >> > >>> >>> >> > Martijn, the tricky situation here is that everyone on the team >>> - >>> >>> and >>> >>> >> > I'm pretty sure including the ones who didn't vote - is very >>> much >>> >>> >> > against having this constructor. VERY much. It leads to a >>> dangerous >>> >>> >> > situation and as a message earlier this week showed, people ARE >>> >>> using >>> >>> >> > it the wrong way. God knows how many. This is not just a matter >>> of >>> >>> not >>> >>> >> > reading the docs for people, this is a matter of the API >>> guiding >>> in >>> >>> >> > the wrong direction. Also, if documentation was enough, why >>> think >>> >>> >> > about API design in the first place? Also, comparing with >>> methods >>> >>> that >>> >>> >> > have the 'do not use this' warnings in them is just plain bs, >>> as >>> we >>> >>> >> > have such 'constructs' because there was alternative to them, >>> >>> mainly >>> >>> >> > because Java doesn't have a friends construction. >>> >>> >> > >>> >>> >> >> -1 on removing the constructor. Just as Jonathan... >>> >>> >> > >>> >>> >> > Great, another veto. Jonathan was against removing the class >>> >>> >> > altogether (which I am personally +1 for, but not that strongly >>> to >>> >>> >> > make a lot of trouble). He stated he uses the IPageLink >>> constructor >>> >>> a >>> >>> >> > lot, while this vote is for removing the Page constructor. >>> >>> >> > >>> >>> >> > The only alternative I can see here is to leave the constructor >>> in >>> >>> - >>> >>> a >>> >>> >> > half baked solution which I would hate nevertheless - but put a >>> >>> >> > @deprecated tag with a big fat warning in it. >>> >>> >> > >>> >>> >> > Regards, >>> >>> >> > >>> >>> >> > Eelco >>> >>> >> > >>> >>> >> > >>> >>> >> >>> >>> >> -- >>> >>> >> View this message in context: >>> >>> >> >>> >>> >>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9145872 >>> >>> >> Sent from the Wicket - Dev mailing list archive at Nabble.com. >>> >>> >> >>> >>> >> >>> >>> > >>> >>> > >>> >>> >>> >>> -- >>> >>> View this message in context: >>> >>> >>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9147112 >>> >>> Sent from the Wicket - Dev mailing list archive at Nabble.com. >>> >>> >>> >>> >>> >> >>> >> >>> > >>> > >>> >>> -- >>> View this message in context: >>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9149820 >>> Sent from the Wicket - Dev mailing list archive at Nabble.com. >>> >>> >> >> > > -- View this message in context: http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9150708 Sent from the Wicket - Dev mailing list archive at Nabble.com.