Re: [Vote] VOTE: remove PageLink(String,Page) constructor
hey, i'm USING this! you will break a ton of my code for no reason. the IPageLink constructor is useful for delayed construction with interesting parameters and also fits neatly in with the way i do navigations (the page identity bit). i'm very -1 for removing this. if you really MUST mess with the api because it doesn't fit your style, can we at least keep for people like me who are using it or like it and rename to DelayedPageLink or something? Matej Knopp wrote: > > Frank Bille wrote: >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: >>> >>> why dont we just remove PageLink altogether? >>> >> >> +1, remove the bastard :) >> >> Frank >> > a big +1 from me too. > > -Matej > > -- View this message in context: http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
BookmarkableLink and PageLink are different things. They do generate different urls. (and maybe some security things can be easier applied to a pagelink then on a bookmarkable page which exposes the page class directly) So i would say just remove the Page constructor. johan On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: hey, i'm USING this! you will break a ton of my code for no reason. the IPageLink constructor is useful for delayed construction with interesting parameters and also fits neatly in with the way i do navigations (the page identity bit). i'm very -1 for removing this. if you really MUST mess with the api because it doesn't fit your style, can we at least keep for people like me who are using it or like it and rename to DelayedPageLink or something? Matej Knopp wrote: > > Frank Bille wrote: >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: >>> >>> why dont we just remove PageLink altogether? >>> >> >> +1, remove the bastard :) >> >> Frank >> > a big +1 from me too. > > -Matej > > -- View this message in context: http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
Ok. As long as the page constructor goes. That one is too dangerous and sweat starts breaking out all over when I think of the number of people that may be mis-using this class already. Eelco On 2/23/07, Johan Compagner <[EMAIL PROTECTED]> wrote: BookmarkableLink and PageLink are different things. They do generate different urls. (and maybe some security things can be easier applied to a pagelink then on a bookmarkable page which exposes the page class directly) So i would say just remove the Page constructor. johan On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: > > > > hey, i'm USING this! you will break a ton of my code for no reason. > > the IPageLink constructor is useful for delayed construction with > interesting parameters and also fits neatly in with the way i do > navigations > (the page identity bit). > > i'm very -1 for removing this. > > if you really MUST mess with the api because it doesn't fit your style, > can > we at least keep for people like me who are using it or like it and rename > to DelayedPageLink or something? > > > Matej Knopp wrote: > > > > Frank Bille wrote: > >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: > >>> > >>> why dont we just remove PageLink altogether? > >>> > >> > >> +1, remove the bastard :) > >> > >> Frank > >> > > a big +1 from me too. > > > > -Matej > > > > > > -- > View this message in context: > http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 > Sent from the Wicket - Dev mailing list archive at Nabble.com. > >
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
yep, its all about my style -igor On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: hey, i'm USING this! you will break a ton of my code for no reason. the IPageLink constructor is useful for delayed construction with interesting parameters and also fits neatly in with the way i do navigations (the page identity bit). i'm very -1 for removing this. if you really MUST mess with the api because it doesn't fit your style, can we at least keep for people like me who are using it or like it and rename to DelayedPageLink or something? Matej Knopp wrote: > > Frank Bille wrote: >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: >>> >>> why dont we just remove PageLink altogether? >>> >> >> +1, remove the bastard :) >> >> Frank >> > a big +1 from me too. > > -Matej > > -- View this message in context: http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
yes. i'm +1 on removing that constructor for user education purposes. it's good to prevent innocent people from doing something naive here. Eelco Hillenius wrote: > > Ok. As long as the page constructor goes. That one is too dangerous > and sweat starts breaking out all over when I think of the number of > people that may be mis-using this class already. > > Eelco > > On 2/23/07, Johan Compagner <[EMAIL PROTECTED]> wrote: >> BookmarkableLink and PageLink are different things. >> They do generate different urls. (and maybe some security things >> can be easier applied to a pagelink then on a bookmarkable page >> which exposes the page class directly) >> >> So i would say just remove the Page constructor. >> >> johan >> >> On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: >> > >> > >> > >> > hey, i'm USING this! you will break a ton of my code for no reason. >> > >> > the IPageLink constructor is useful for delayed construction with >> > interesting parameters and also fits neatly in with the way i do >> > navigations >> > (the page identity bit). >> > >> > i'm very -1 for removing this. >> > >> > if you really MUST mess with the api because it doesn't fit your style, >> > can >> > we at least keep for people like me who are using it or like it and >> rename >> > to DelayedPageLink or something? >> > >> > >> > Matej Knopp wrote: >> > > >> > > Frank Bille wrote: >> > >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: >> > >>> >> > >>> why dont we just remove PageLink altogether? >> > >>> >> > >> >> > >> +1, remove the bastard :) >> > >> >> > >> Frank >> > >> >> > > a big +1 from me too. >> > > >> > > -Matej >> > > >> > > >> > >> > -- >> > View this message in context: >> > >> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 >> > 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#a9122967 Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
Very convenient to hold a vote when I'm not around... -1 on removing the constructor. Just as Jonathan, I have tons of code that relies on that constructor. If the reason is that people don't read documentation, then we can scrap about 100% of our internal api, you know, where it reads "DON'T CALL THIS", or "DON'T OVERRIDE THIS'? Why hasn't anyone proposed to write an example that displays this problem to our new users? Make it VERY clear that it is not the way to link? Education is writing documentation too, not just removing API because it can be used wrong. Martijn On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: yes. i'm +1 on removing that constructor for user education purposes. it's good to prevent innocent people from doing something naive here. Eelco Hillenius wrote: > > Ok. As long as the page constructor goes. That one is too dangerous > and sweat starts breaking out all over when I think of the number of > people that may be mis-using this class already. > > Eelco > > On 2/23/07, Johan Compagner <[EMAIL PROTECTED]> wrote: >> BookmarkableLink and PageLink are different things. >> They do generate different urls. (and maybe some security things >> can be easier applied to a pagelink then on a bookmarkable page >> which exposes the page class directly) >> >> So i would say just remove the Page constructor. >> >> johan >> >> On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: >> > >> > >> > >> > hey, i'm USING this! you will break a ton of my code for no reason. >> > >> > the IPageLink constructor is useful for delayed construction with >> > interesting parameters and also fits neatly in with the way i do >> > navigations >> > (the page identity bit). >> > >> > i'm very -1 for removing this. >> > >> > if you really MUST mess with the api because it doesn't fit your style, >> > can >> > we at least keep for people like me who are using it or like it and >> rename >> > to DelayedPageLink or something? >> > >> > >> > Matej Knopp wrote: >> > > >> > > Frank Bille wrote: >> > >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: >> > >>> >> > >>> why dont we just remove PageLink altogether? >> > >>> >> > >> >> > >> +1, remove the bastard :) >> > >> >> > >> Frank >> > >> >> > > a big +1 from me too. >> > > >> > > -Matej >> > > >> > > >> > >> > -- >> > View this message in context: >> > >> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 >> > 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#a9122967 Sent from the Wicket - Dev mailing list archive at Nabble.com. -- Learn Wicket at ApacheCon Europe: http://apachecon.com Join the wicket community at irc.freenode.net: ##wicket Wicket 1.2.5 will keep your server alive. Download Wicket now! http://wicketframework.org
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
And by example I mean: a document that is on our website, not something in wicket-examples. Martijn On 2/25/07, Martijn Dashorst <[EMAIL PROTECTED]> wrote: Very convenient to hold a vote when I'm not around... -1 on removing the constructor. Just as Jonathan, I have tons of code that relies on that constructor. If the reason is that people don't read documentation, then we can scrap about 100% of our internal api, you know, where it reads "DON'T CALL THIS", or "DON'T OVERRIDE THIS'? Why hasn't anyone proposed to write an example that displays this problem to our new users? Make it VERY clear that it is not the way to link? Education is writing documentation too, not just removing API because it can be used wrong. Martijn On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: > > > yes. i'm +1 on removing that constructor for user education purposes. > it's good to prevent innocent people from doing something naive here. > > > Eelco Hillenius wrote: > > > > Ok. As long as the page constructor goes. That one is too dangerous > > and sweat starts breaking out all over when I think of the number of > > people that may be mis-using this class already. > > > > Eelco > > > > On 2/23/07, Johan Compagner <[EMAIL PROTECTED]> wrote: > >> BookmarkableLink and PageLink are different things. > >> They do generate different urls. (and maybe some security things > >> can be easier applied to a pagelink then on a bookmarkable page > >> which exposes the page class directly) > >> > >> So i would say just remove the Page constructor. > >> > >> johan > >> > >> On 2/23/07, Jonathan Locke <[EMAIL PROTECTED]> wrote: > >> > > >> > > >> > > >> > hey, i'm USING this! you will break a ton of my code for no reason. > >> > > >> > the IPageLink constructor is useful for delayed construction with > >> > interesting parameters and also fits neatly in with the way i do > >> > navigations > >> > (the page identity bit). > >> > > >> > i'm very -1 for removing this. > >> > > >> > if you really MUST mess with the api because it doesn't fit your style, > >> > can > >> > we at least keep for people like me who are using it or like it and > >> rename > >> > to DelayedPageLink or something? > >> > > >> > > >> > Matej Knopp wrote: > >> > > > >> > > Frank Bille wrote: > >> > >> On 2/22/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote: > >> > >>> > >> > >>> why dont we just remove PageLink altogether? > >> > >>> > >> > >> > >> > >> +1, remove the bastard :) > >> > >> > >> > >> Frank > >> > >> > >> > > a big +1 from me too. > >> > > > >> > > -Matej > >> > > > >> > > > >> > > >> > -- > >> > View this message in context: > >> > > >> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9115685 > >> > 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#a9122967 > Sent from the Wicket - Dev mailing list archive at Nabble.com. > > -- Learn Wicket at ApacheCon Europe: http://apachecon.com Join the wicket community at irc.freenode.net: ##wicket Wicket 1.2.5 will keep your server alive. Download Wicket now! http://wicketframework.org -- Learn Wicket at ApacheCon Europe: http://apachecon.com Join the wicket community at irc.freenode.net: ##wicket Wicket 1.2.5 will keep your server alive. Download Wicket now! http://wicketframework.org
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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) What's the advantage of have the class one anyway actually? As far as I can see, such use is only possible with pages with public default constructors or constructors with a page parameters argument - in other words, bookmarkable pages. This link would then create an internal link, which has no advantage in this case over bookmarkable links; it forces pages to be kept in the page map, even if they are stateless (and also forces session creation as a result of that). And I don't think many if any people are using exotic versions of IPageFactory to create non-bookmarkable page instances based on the class argument. delayedpagelink (the one with the ipagelink constructor) I like the name PageLink much better tbh. If PageLink would only have the IPageLink parameter, it's usage is obvious and I think PageLink is an easier to discover name than DelayedPageLink. My 2c, Eelco
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
ok. i'll buy that for a dollar. Eelco Hillenius wrote: > > 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) >> > > What's the advantage of have the class one anyway actually? As far as > I can see, such use is only possible with pages with public default > constructors or constructors with a page parameters argument - in > other words, bookmarkable pages. This link would then create an > internal link, which has no advantage in this case over bookmarkable > links; it forces pages to be kept in the page map, even if they are > stateless (and also forces session creation as a result of that). And > I don't think many if any people are using exotic versions of > IPageFactory to create non-bookmarkable page instances based on the > class argument. > >> >> delayedpagelink (the one with the ipagelink constructor) > > I like the name PageLink much better tbh. If PageLink would only have > the IPageLink parameter, it's usage is obvious and I think PageLink is > an easier to discover name than DelayedPageLink. > > My 2c, > > Eelco > > -- View this message in context: http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9146370 Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
ok. i'll buy that for a dollar. I'll get you the next coffee then ;) Eelco
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
The class constructor is also a delayed creation.. just as the ipagelink.. I would just keep the PageLink with the 2 constructors.. (removing the Page constructor is fine with me but if it is veto's its also fine. then just update the doc with a big warning) johan 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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
Seems way too easy, but what about changing PageLink.linksTo(Page)
from this:
public boolean linksTo(final Page page)
{
return page.getClass() == pageLink.getPageIdentity();
}
to this:
public boolean linksTo(final Class c)
{
return c == pageLink.getPageIdentity();
}
and changing Link.isEnabled() from this:
public boolean isEnabled()
{
if (getAutoEnable())
{
return !linksTo(getPage());
}
return super.isEnabled();
}
to this:
public boolean isEnabled()
{
if (getAutoEnable())
{
return !linksTo(getPage().getClass());
}
return super.isEnabled();
}
(based on 1.2.x)
-Ryan
On Feb 25, 2007, at 11:38 AM, Jonathan Locke 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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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.
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
i think it's good the way it is. i'm loathe to narrow to a class here.
conceptually it's really the page you want to check against. not
necessarily
just the page's class.
Ryan Holmes wrote:
>
> Seems way too easy, but what about changing PageLink.linksTo(Page)
> from this:
>
> public boolean linksTo(final Page page)
> {
> return page.getClass() == pageLink.getPageIdentity();
> }
>
> to this:
>
> public boolean linksTo(final Class c)
> {
> return c == pageLink.getPageIdentity();
> }
>
>
> and changing Link.isEnabled() from this:
>
> public boolean isEnabled()
> {
> if (getAutoEnable())
> {
> return !linksTo(getPage());
> }
> return super.isEnabled();
> }
>
> to this:
> public boolean isEnabled()
> {
> if (getAutoEnable())
> {
> return !linksTo(getPage().getClass());
> }
> return super.isEnabled();
> }
>
> (based on 1.2.x)
>
> -Ryan
>
> On Feb 25, 2007, at 11:38 AM, Jonathan Locke 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
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
let's just take a step back and ask: how can we make this less work?
could new AutoDisableLink 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
>>> >> >
>>> >> >
>>> >>
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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 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
>>> -
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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 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.
>> >
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
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 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
>>> >>> >> pageclassli
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
On 2/26/07, Martijn Dashorst <[EMAIL PROTECTED]> wrote: I see it as a pretty major api break. Considering that everyone on our project uses the construct to return quickly to the previous page, which is a very valid usecase. I can only imagine that many others have used this construct as well. I *really* think that removing *existing* api because it /can/ be used incorrectly instead of educating through documentation is not the way to go. We disagree then. And it is unfortunately not just /can/ but rather /is/ and /will be/. I am also against renaming it, again because the API break is significant. I also don't see a problem with deprecating the constructor and or class, and removing it in 1.4: this gives our projects, and those of our users with *LEGITIMATE* uses of it, the chance to migrate. It will also give users with wrong uses the message that they are using it wrong and a chance to do somethign about it at a moment that is convenient for them. As for 2.0 I don't mind removing it as the API is already much different (due to the constructor change). Let's go that path then. Deprecation is there for a reason: API migrations. I think we have a valid usecase here and should use that instead of blatently removing any constructor/class that is/may be widely used. Unfortunately, it often takes forever to remove deprecations, and users can just happily use them and still get pissed off when someday they are removed. I like to break early when we can. But well, that is a discussion that is more generic than needed for this thread. As for the veto, I think we all have a right to disagree here: that is what I'm doing. Since when is rubber stamping proposals 'the Apache way'? Sure you have that right as anyone else in the team has. I hope you don't take it too lightly though; too much use of veto's can cripple any group process. We have always respected your vetos, even before we were trying to play by the Apache rules, and we will in the future. But vetos are usually pretty frustrating for the other's that are veto-ed away, so don't expect me (or anyone else) to be cheery about it. Eelco
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
I see it as a pretty major api break. Considering that everyone on our project uses the construct to return quickly to the previous page, which is a very valid usecase. I can only imagine that many others have used this construct as well. I *really* think that removing *existing* api because it /can/ be used incorrectly instead of educating through documentation is not the way to go. I am also against renaming it, again because the API break is significant. I also don't see a problem with deprecating the constructor and or class, and removing it in 1.4: this gives our projects, and those of our users with *LEGITIMATE* uses of it, the chance to migrate. It will also give users with wrong uses the message that they are using it wrong and a chance to do somethign about it at a moment that is convenient for them. As for 2.0 I don't mind removing it as the API is already much different (due to the constructor change). Deprecation is there for a reason: API migrations. I think we have a valid usecase here and should use that instead of blatently removing any constructor/class that is/may be widely used. As for the veto, I think we all have a right to disagree here: that is what I'm doing. Since when is rubber stamping proposals 'the Apache way'? Martijn On 2/25/07, Eelco Hillenius <[EMAIL PROTECTED]> 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 incluI sading 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 -- Learn Wicket at ApacheCon Europe: http://apachecon.com Join the wicket community at irc.freenode.net: ##wicket Wicket 1.2.5 will keep your server alive. Download Wicket now! http://wicketframework.org
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
* Martijn Dashorst: > I see it as a pretty major api break. Considering that everyone > on our project uses the construct to return quickly to the > previous page, which is a very valid usecase. To be sure that there is no misunderstanding, can you provide the exact source code that illustrates your opinion please? -- Jean-Baptiste Quenot aka John Banana Qwerty http://caraldi.com/jbq/
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
instead of doing this:
PageLink link = new PageLink("previouspage", prevPage);
you have to do:
final Page prevPage = xx;
PageLink link = new PageLink("previouspage", new IPageLink()
{
private static final long serialVersionUID = 1L;
public Page getPage()
{
// Create page using page factory
return prevPage;
}
public Class getPageIdentity()
{
return prevPage.getClass();
}
});
johan
On 2/27/07, Jean-Baptiste Quenot <[EMAIL PROTECTED]> wrote:
* Martijn Dashorst:
> I see it as a pretty major api break. Considering that everyone
> on our project uses the construct to return quickly to the
> previous page, which is a very valid usecase.
To be sure that there is no misunderstanding, can you provide the
exact source code that illustrates your opinion please?
--
Jean-Baptiste Quenot
aka John Banana Qwerty
http://caraldi.com/jbq/
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
Or just make the base class once for your project with a similar
constructor and replace all instances. I don't know about you guys,
but it'll take me less than 10 minutes to do that for a whole project,
and thanks to Java and the actual removal of the constructor,
risk-free.
Eelco
On 2/27/07, Johan Compagner <[EMAIL PROTECTED]> wrote:
instead of doing this:
PageLink link = new PageLink("previouspage", prevPage);
you have to do:
final Page prevPage = xx;
PageLink link = new PageLink("previouspage", new IPageLink()
{
private static final long serialVersionUID = 1L;
public Page getPage()
{
// Create page using page factory
return prevPage;
}
public Class getPageIdentity()
{
return prevPage.getClass();
}
});
johan
On 2/27/07, Jean-Baptiste Quenot <[EMAIL PROTECTED]> wrote:
>
> * Martijn Dashorst:
>
> > I see it as a pretty major api break. Considering that everyone
> > on our project uses the construct to return quickly to the
> > previous page, which is a very valid usecase.
>
> To be sure that there is no misunderstanding, can you provide the
> exact source code that illustrates your opinion please?
> --
> Jean-Baptiste Quenot
> aka John Banana Qwerty
> http://caraldi.com/jbq/
>
Re: [Vote] VOTE: remove PageLink(String,Page) constructor
if we're getting rid of PageLink entirely, you mean:
new Link("previousPage")
{
public void onClick() { setResponsePage(prevPage); }
public boolean linksTo(Page page) { return page.equals(prevPage); }
}
Johan Compagner wrote:
>
> instead of doing this:
>
> PageLink link = new PageLink("previouspage", prevPage);
>
> you have to do:
>
> final Page prevPage = xx;
> PageLink link = new PageLink("previouspage", new IPageLink()
> {
> private static final long serialVersionUID = 1L;
>
> public Page getPage()
> {
> // Create page using page factory
> return prevPage;
> }
>
> public Class getPageIdentity()
> {
> return prevPage.getClass();
> }
> });
>
> johan
>
>
> On 2/27/07, Jean-Baptiste Quenot <[EMAIL PROTECTED]> wrote:
>>
>> * Martijn Dashorst:
>>
>> > I see it as a pretty major api break. Considering that everyone
>> > on our project uses the construct to return quickly to the
>> > previous page, which is a very valid usecase.
>>
>> To be sure that there is no misunderstanding, can you provide the
>> exact source code that illustrates your opinion please?
>> --
>> Jean-Baptiste Quenot
>> aka John Banana Qwerty
>> http://caraldi.com/jbq/
>>
>
>
--
View this message in context:
http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9186573
Sent from the Wicket - Dev mailing list archive at Nabble.com.
