I've made all the recommended changes. LinkTagSupport reverted back to its previous form except that I added the required Locale argument to the UrlBuilder constructor. Please give this stuff a whirl and let me know if you have any problems.
-Ben Tim Fennell wrote: > Ah, I see. I hadn't looked that far and had forgotten that the > UrlBuilder slaps things into the StringBuilder as it goes instead of > caching them and building at the end. > > Yes, I think we should add the Locale to the constructor. I think > it's fair to leave the old constructor for now, and deprecate it - > since calling the Formatters with the default system locale would > mirror fairly closely the current UrlBuilder behaviour of toStringing > () the parameters. > > -t > > On May 29, 2007, at 10:29 AM, Ben Gunter wrote: > > >> Yes, that's all it needs, but the only unanswered question now is how >> will UrlBuilder receive the locale from the caller? It currently >> has no >> knowledge of the locale. Looking at how UrlBuilder works -- appending >> each parameter to a StringBuilder as it is added -- it seems the only >> logical approach is to overload the constructor >> >> public UrlBuilder(String url, boolean isForPage) >> >> to look like >> >> public UrlBuilder(Locale locale, String url, boolean isForPage) >> >> So I guess I'm just making sure that's what you had in mind. If so, >> should I deprecate the old constructor and change it to use the >> default >> locale? >> >> -Ben >> >> Tim Fennell wrote: >> >>> Sorry, yes. That we should add the functionality to UrlBuilder so >>> that it's re-usable, and then force users to provide the Locale at >>> some point. >>> >>> You'd mentioned that the problem was that the OnwardResolution didn't >>> have access to the Locale/Request, and I was trying to point out that >>> that /should/ be a pretty easy fix, by adding OnwardResolution.getUrl >>> (Locale), and passing the Locale through to the UrlBuilder - that's >>> all it needs from the request right? >>> >>> -t >>> >>> On May 29, 2007, at 10:02 AM, Ben Gunter wrote: >>> >>> >>> >>>> I'm a little confused. Are you saying we should add >>>> OnwardResolution.getUrl(Locale) but move the stuff that does the >>>> formatting into UrlBuilder? That would also require a change to >>>> UrlBuilder because it doesn't use a Locale in its current form. >>>> Should >>>> we just add a UrlBuilder.setLocale(Locale)? New constructor? >>>> >>>> Tim Fennell wrote: >>>> >>>> >>>>> So I just took a look at the code in SVN, and I think it's >>>>> definitely >>>>> doable to make this work in UrlBuilder. >>>>> >>>>> My suggestion would be that we add a method to OnwardResolution >>>>> that >>>>> looks like: >>>>> String getUrl(Locale local); >>>>> >>>>> And that we deprecate the existing getUrl(), and have it call >>>>> through >>>>> with the system default locale. We should probably also change >>>>> both >>>>> of these methods to protected (I'm not sure why I made them >>>>> public). >>>>> The only places getUrl() is called right now are in subclasses, >>>>> inside >>>>> the execute() method which has access to both the request and >>>>> response, and could therefore supply the Locale easily enough. >>>>> >>>>> -t >>>>> >>>>> On May 29, 2007, at 9:40 AM, Barry Davies wrote: >>>>> >>>>> >>>>> >>>>>> Ben, I ran into the exact same problem when I tried to prepare a >>>>>> patch for the issue. I posted to the boards with the exact same >>>>>> question, but didn't get alot of replies. I was thinking about >>>>>> creating a ThreadLocal registry for the Locale (or maybe even the >>>>>> entire session), and exposing that as another convenience >>>>>> feature of >>>>>> Stripes. But I'm not a strict test-driven development kinda >>>>>> guy, so >>>>>> I didn't know whether that was the sort of thing that would cause >>>>>> Tim >>>>>> to do the Invasion of the Body Snatchers point-and-gutteral-noise >>>>>> <http://www.youtube.com/watch?v=na2W38tLp_Q> (1:50 into the clip) >>>>>> when he saw my patch. I'm still leaning toward thinking it'd be a >>>>>> good idea to expose a ThreadLocal Session Registry from Stripes >>>>>> as a >>>>>> convenience feature, and it would assist in the resolution of this >>>>>> issue. I understand that this registry would become something >>>>>> that >>>>>> ever single test would have to mock into existence, because you >>>>>> never >>>>>> know when something is using it. What do you think? >>>>>> >>>>>> -BD aka RJ >>>>>> >>>>>> ----- Original Message ---- >>>>>> From: Ben Gunter <[EMAIL PROTECTED] <mailto:[EMAIL PROTECTED]>> >>>>>> To: Stripes Development List >>>>>> <[email protected] >>>>>> <mailto:[email protected]>> >>>>>> Sent: Monday, May 28, 2007 11:14:37 PM >>>>>> Subject: Re: [Stripes-dev] stripes:param should use formatters >>>>>> when >>>>>> possible >>>>>> >>>>>> There's a small problem. I made this change for s:url and >>>>>> s:link. I >>>>>> did so by changing LinkTagSupport.buildUrl() to copy the parameter >>>>>> map, format each value, and then call UrlBuilder.addParameters(). >>>>>> >>>>>> I know I could have changed UrlBuilder.addParameter[s] to do the >>>>>> formatting. I opted not to because I figured it might be better to >>>>>> let UrlBuilder remain more of a utility class that has no >>>>>> Stripes-specific dependencies. I don't know if that's a valid >>>>>> reason. >>>>>> If y'all think it might be better to have UrlBuilder do the >>>>>> formatting then I can change it easily enough. >>>>>> >>>>>> The reason it might be good to do it in UrlBuilder is because it >>>>>> could be done once and fix s:link, s:url, and OnwardResolution >>>>>> all at >>>>>> once. But I went to fix OnwardResolution and realized that it >>>>>> doesn't >>>>>> have access to the Locale. The tags can get it from >>>>>> pageContext.getRequest().getLocale(), but the OnwardResolution >>>>>> doesn't have a reference to a request object and so can't get the >>>>>> Locale to pass into Formatter.format(). Any suggestions on how to >>>>>> get >>>>>> around this? >>>>>> >>>>>> -Ben >>>>>> >>>>>> Tim Fennell wrote: >>>>>> >>>>>> >>>>>>> Yes, I agree. >>>>>>> -t >>>>>>> >>>>>>> On May 28, 2007, at 3:08 PM, Barry Davies wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Agreed, having <s:param> (or <s:link>) use formatters by default >>>>>>>> sounds like the most dev-friendly option to me. Of course, >>>>>>>> all of >>>>>>>> this still leaves out the fact that >>>>>>>> OnwardResolution.addParameter >>>>>>>> also doesn't use the formatters. Formatters-in-param-tag was >>>>>>>> mentioned also in STS-322 <http://mc4j.org/jira/browse/STS-322>, >>>>>>>> that I logged awhile ago. Does anyone else agrees that there >>>>>>>> should be a symmetry between <s:param> and >>>>>>>> OnwardResolution.addParameter()? >>>>>>>> -BD aka RJ >>>>>>>> >>>>>>>> ----- Original Message ---- >>>>>>>> From: Tim Fennell <[EMAIL PROTECTED] <mailto:[EMAIL PROTECTED]>> >>>>>>>> To: Stripes Development List >>>>>>>> <[email protected] >>>>>>>> <mailto:[email protected]>> >>>>>>>> Sent: Monday, May 28, 2007 8:37:00 AM >>>>>>>> Subject: Re: [Stripes-dev] stripes:param should use formatters >>>>>>>> when >>>>>>>> possible >>>>>>>> >>>>>>>> I think I'm a bit late to the game on this one, since the commit >>>>>>>> email have already flown. I definitely like the s:format tag >>>>>>>> - I >>>>>>>> think it's a great addition. >>>>>>>> >>>>>>>> But I'm also wondering if it wouldn't be best to use the >>>>>>>> formatting >>>>>>>> engine, with default style/pattern information, as the >>>>>>>> default way >>>>>>>> of rendering objects to Strings in the param tags? I can't >>>>>>>> really >>>>>>>> think of a reason that this would be worse than the current way >>>>>>>> (just toString()ing things), but it would also make life >>>>>>>> easier in >>>>>>>> a lot of cases. >>>>>>>> >>>>>>>> I think the major reason this was brought up was in the >>>>>>>> Stripernate >>>>>>>> (or similar) pattern where you might have links like: >>>>>>>> <s:link href="/Edit.action"> >>>>>>>> <s:param name="id" value="${person.id}"/> >>>>>>>> </s:slink> >>>>>>>> >>>>>>>> If I recall correctly, then folks ended up implementing a lot of >>>>>>>> toString() methods to return IDs - and that's less than ideal >>>>>>>> for >>>>>>>> several reasons. The new solution (with format tag) would >>>>>>>> obviously work here, but it starts to get a bit verbose and >>>>>>>> repetitive if you don't really need to provide the additional >>>>>>>> formatting parameters: >>>>>>>> <s:link href="/Edit.action"> >>>>>>>> <s:param name="id"><s:fomat value="${person.id}"/></ >>>>>>>> s:param> >>>>>>>> </s:slink> >>>>>>>> >>>>>>>> So what do you think about using the formatters with their >>>>>>>> default >>>>>>>> configurations as the default way to turn Object->String in the >>>>>>>> param tag (actually, probably s:link tag)? >>>>>>>> >>>>>>>> -t >>>>>>>> >>>>>>>> >>>>>>>> On May 27, 2007, at 1:45 PM, Jeppe Cramon wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> Hi >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Option 1 sounds good and is as Kai says useful in other >>>>>>>>> circumstances too :) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> /Jepe >>>>>>>>> >>>>>>>>> >>>>>>>>> --------------------------------------------------------------- >>>>>>>>> -- >>>>>>>>> ------- >>>>>>>>> >>>>>>>>> *From:* [EMAIL PROTECTED] >>>>>>>>> <mailto:[EMAIL PROTECTED]> >>>>>>>>> [mailto:[EMAIL PROTECTED] *On >>>>>>>>> Behalf Of *Ben Gunter >>>>>>>>> *Sent:* 26. maj 2007 19:03 >>>>>>>>> *To:* Stripes Development List >>>>>>>>> *Subject:* Re: [Stripes-dev] stripes:param should use >>>>>>>>> formatters >>>>>>>>> when possible >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> I knew there had to be more to this story :) >>>>>>>>> >>>>>>>>> As it turns out, if you use s:param in the form: >>>>>>>>> >>>>>>>>> <s:param name="x">${someObject}</s:param> >>>>>>>>> >>>>>>>>> Then the tag's body content is only available as a String. >>>>>>>>> There's >>>>>>>>> no way to apply a formatter to it because it gets evaluated >>>>>>>>> ahead >>>>>>>>> of time by the JSP engine. On the other hand... >>>>>>>>> >>>>>>>>> <s:param name="x" value="${someObject}" /> >>>>>>>>> >>>>>>>>> Does pass the Object into setValue so a Formatter could be used >>>>>>>>> there. The problem with that is that the s:param tag is >>>>>>>>> expected >>>>>>>>> to pass the original Object value to its parent tag, not >>>>>>>>> necessarily just a String. That means it's up to the parent >>>>>>>>> tag to >>>>>>>>> format it correctly. And the problem with /that/ is that some >>>>>>>>> formatters require a format pattern and/or format type, and >>>>>>>>> that >>>>>>>>> bit of information might be different for each parameter. >>>>>>>>> >>>>>>>>> So we're left with a few choices... >>>>>>>>> >>>>>>>>> 1. Add a new tag <s:format value="" [var=""] [formatType=""] >>>>>>>>> [formatPattern=""] />. One could then use it like >>>>>>>>> <s:param >>>>>>>>> name="x"><s:format value="${someObject}" /></s:param> >>>>>>>>> 2. Add some new attributes to s:param, such as >>>>>>>>> format="true", >>>>>>>>> formatType="whatever" and formatPattern="whatever". >>>>>>>>> format="true" would be assumed if either formatType or >>>>>>>>> formatPattern were non-null. If formatType and >>>>>>>>> formatPattern >>>>>>>>> are not applicable for a given parameter, then >>>>>>>>> format="true" >>>>>>>>> would be required as the default would be false. >>>>>>>>> 3. Do nothing. I don't like this idea :) >>>>>>>>> >>>>>>>>> I really like option (1). This would be a good idea because >>>>>>>>> it is >>>>>>>>> useful in many different situations and allows developers to >>>>>>>>> take >>>>>>>>> full advantage of Stripes' formatting capabilities from within >>>>>>>>> their JSPs. Imagine this, given a Person class with >>>>>>>>> firstName and >>>>>>>>> lastName properties: >>>>>>>>> >>>>>>>>> * <s:format formatType="lastNameFirst" value="${person}" / >>>>>>>>> >>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> >>>>>>>>>> >>>>>>>>> Gunter, Ben >>>>>>>>> * <s:format formatType="normal" value="${person}" /> -- >>>>>>>>> Ben Gunter >>>>>>>>> * <s:format value="${person}" /> -- the primary key of the >>>>>>>>> object, which is the default format >>>>>>>>> >>>>>>>>> >>>>>>>>> I'll do this if nobody is opposed to the idea. It solves the >>>>>>>>> s:param problem nicely and provides lots of other useful >>>>>>>>> capabilities. >>>>>>>>> >>>>>>>>> -Ben >>>>>>>>> >>>>>>>>> Jeppe Cramon wrote: >>>>>>>>> >>>>>>>>> No, I agree :) >>>>>>>>> >>>>>>>>> /Jeppe >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: [EMAIL PROTECTED] >>>>>>>>>> <mailto:[EMAIL PROTECTED]> >>>>>>>>>> [mailto:stripes- >>>>>>>>>> [EMAIL PROTECTED] <mailto:development- >>>>>>>>>> [EMAIL PROTECTED]>] On Behalf Of Ben Gunter >>>>>>>>>> Sent: 26. maj 2007 02:38 >>>>>>>>>> To: Stripes Development List >>>>>>>>>> Subject: [Stripes-dev] stripes:param should use formatters >>>>>>>>>> when possible >>>>>>>>>> >>>>>>>>>> Does anybody disagree with this? >>>>>>>>>> >>>>>>>>>> http://mc4j.org/jira/browse/STS-374 >>>>>>>>>> >>>>>>>>>> -------------------------------------------------------------- >>>>>>>>>> -- >>>>>>>>>> --------- >>>>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>>>> Download DB2 Express C - the FREE version of DB2 express and >>>>>>>>>> take >>>>>>>>>> control of your XML. No limits. Just data. Click to get it >>>>>>>>>> now. >>>>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>>>> _______________________________________________ >>>>>>>>>> Stripes-development mailing list >>>>>>>>>> [email protected] <mailto:Stripes- >>>>>>>>>> [EMAIL PROTECTED]> >>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes- >>>>>>>>>> development >>>>>>>>>> >>>>>>>>>> __________ NOD32 2292 (20070525) Information >>>>>>>>>> __________ >>>>>>>>>> >>>>>>>>>> This message was checked by NOD32 antivirus system. >>>>>>>>>> http://www.eset.com >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> --------------------------------------------------------------- >>>>>>>>> -- >>>>>>>>> -------- >>>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>>> Download DB2 Express C - the FREE version of DB2 express and >>>>>>>>> take >>>>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>>> _______________________________________________ >>>>>>>>> Stripes-development mailing list >>>>>>>>> [email protected] <mailto:Stripes- >>>>>>>>> [EMAIL PROTECTED]> >>>>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes- >>>>>>>>> development >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> __________ NOD32 2292 (20070525) Information __________ >>>>>>>>> >>>>>>>>> This message was checked by NOD32 antivirus system. >>>>>>>>> http://www.eset.com >>>>>>>>> >>>>>>>>> --------------------------------------------------------------- >>>>>>>>> -- >>>>>>>>> -------- >>>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>>> Download DB2 Express C - the FREE version of DB2 express and >>>>>>>>> take >>>>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>>> _______________________________________________ >>>>>>>>> Stripes-development mailing list >>>>>>>>> [email protected] >>>>>>>>> <mailto:[email protected]> >>>>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes- >>>>>>>>> development >>>>>>>>> >>>>>>>>> >>>>>>>> ---------------------------------------------------------------- >>>>>>>> -- >>>>>>>> ------- >>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>> Download DB2 Express C - the FREE version of DB2 express and >>>>>>>> take >>>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>> _______________________________________________ >>>>>>>> Stripes-development mailing list >>>>>>>> [email protected] >>>>>>>> <mailto:[email protected]> >>>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>>>>> >>>>>>>> >>>>>>>> ---------------------------------------------------------------- >>>>>>>> -- >>>>>>>> ------ >>>>>>>> It's here! Your new message! >>>>>>>> Get new email alerts >>>>>>>> <http://us.rd.yahoo.com/evt=49938/*http:// >>>>>>>> tools.search.yahoo.com/toolbar/features/mail/> >>>>>>>> with the free Yahoo! Toolbar. >>>>>>>> ---------------------------------------------------------------- >>>>>>>> -- >>>>>>>> ------- >>>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>>> Download DB2 Express C - the FREE version of DB2 express and >>>>>>>> take >>>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>>> _______________________________________________ >>>>>>>> Stripes-development mailing list >>>>>>>> [email protected] >>>>>>>> <mailto:[email protected]> >>>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>>>>> >>>>>>>> >>>>>>> ----------------------------------------------------------------- >>>>>>> -- >>>>>>> ----- >>>>>>> >>>>>>> ----------------------------------------------------------------- >>>>>>> -- >>>>>>> ------ >>>>>>> This SF.net email is sponsored by DB2 Express >>>>>>> Download DB2 Express C - the FREE version of DB2 express and take >>>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>>> http://sourceforge.net/powerbar/db2/ >>>>>>> ----------------------------------------------------------------- >>>>>>> -- >>>>>>> ----- >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Stripes-development mailing list >>>>>>> [email protected] >>>>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>>>> >>>>>>> >>>>>>> >>>>>> ------------------------------------------------------------------ >>>>>> -- >>>>>> ----- >>>>>> This SF.net email is sponsored by DB2 Express >>>>>> Download DB2 Express C - the FREE version of DB2 express and take >>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>> http://sourceforge.net/powerbar/db2/ >>>>>> _______________________________________________ >>>>>> Stripes-development mailing list >>>>>> [email protected] >>>>>> <mailto:[email protected]> >>>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>>> >>>>>> >>>>>> ------------------------------------------------------------------ >>>>>> -- >>>>>> ---- >>>>>> Park yourself in front of a world of choices in alternative >>>>>> vehicles. >>>>>> Visit the Yahoo! Auto Green Center. >>>>>> <http://us.rd.yahoo.com/evt=48246/*http://autos.yahoo.com/ >>>>>> green_center/;_ylc=X3oDMTE5cDF2bXZzBF9TAzk3MTA3MDc2BHNlYwNtYWlsdGF >>>>>> nc >>>>>> wRzbGsDZ3JlZW4tY2VudGVy> >>>>>> >>>>>> ------------------------------------------------------------------ >>>>>> -- >>>>>> ----- >>>>>> This SF.net email is sponsored by DB2 Express >>>>>> Download DB2 Express C - the FREE version of DB2 express and take >>>>>> control of your XML. No limits. Just data. Click to get it now. >>>>>> http://sourceforge.net/powerbar/db2/ >>>>>> _______________________________________________ >>>>>> Stripes-development mailing list >>>>>> [email protected] >>>>>> <mailto:[email protected]> >>>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>>> >>>>>> >>>>> ------------------------------------------------------------------- >>>>> -- >>>>> --- >>>>> >>>>> ------------------------------------------------------------------- >>>>> -- >>>>> ---- >>>>> This SF.net email is sponsored by DB2 Express >>>>> Download DB2 Express C - the FREE version of DB2 express and take >>>>> control of your XML. No limits. Just data. Click to get it now. >>>>> http://sourceforge.net/powerbar/db2/ >>>>> ------------------------------------------------------------------- >>>>> -- >>>>> --- >>>>> >>>>> _______________________________________________ >>>>> Stripes-development mailing list >>>>> [email protected] >>>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>>> >>>>> >>>>> >>>> -------------------------------------------------------------------- >>>> -- >>>> --- >>>> This SF.net email is sponsored by DB2 Express >>>> Download DB2 Express C - the FREE version of DB2 express and take >>>> control of your XML. No limits. Just data. Click to get it now. >>>> http://sourceforge.net/powerbar/db2/ >>>> _______________________________________________ >>>> Stripes-development mailing list >>>> [email protected] >>>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>>> >>>> >>> --------------------------------------------------------------------- >>> ---- >>> This SF.net email is sponsored by DB2 Express >>> Download DB2 Express C - the FREE version of DB2 express and take >>> control of your XML. No limits. Just data. Click to get it now. >>> http://sourceforge.net/powerbar/db2/ >>> _______________________________________________ >>> Stripes-development mailing list >>> [email protected] >>> https://lists.sourceforge.net/lists/listinfo/stripes-development >>> >>> >> ---------------------------------------------------------------------- >> --- >> This SF.net email is sponsored by DB2 Express >> Download DB2 Express C - the FREE version of DB2 express and take >> control of your XML. No limits. Just data. Click to get it now. >> http://sourceforge.net/powerbar/db2/ >> _______________________________________________ >> Stripes-development mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/stripes-development >> > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > _______________________________________________ > Stripes-development mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/stripes-development > ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Stripes-development mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/stripes-development
