Added <spring:argument> subtag for message/theme #297
Conversation
Added a new <spring:argument> tag meant for nesting within <spring:message> and <spring:theme>. The tag is based on the <fmt:param> tag and uses conventions found throughout other Spring tags. Ten new unit tests and over a dozen changed unit tests accompany the changes, which are detailed below. Itemized Changes: - Incremented version number of tag libraries to 4.0. - Added <spring:argument> tag to spring.tld. - Updated documentation for message and theme tags in spring.tld. - Added o.s.web.servlet.tags.ArgumentTag class to handle tag. Added 5 unit tests to test behavior. - Added o.s.web.servlet.tags.ArgumentAware to mark tags supporting arguments. - Updated o.s.web.servlet.tags.MessageTag to implement ArgumentAware and permit either nested argument tags or the arguments attribute, but not both. Also updated JavaDoc. Added three unit tests to test behavior. Updated over a dozen other unit tests to test behavior. - Updated o.s.web.servlet.tags.ThemeTag JavaDoc. - Fixed an @link error in the JavaDoc for o.s.web.servlet.tags.UrlTag - o.s.web.servlet.tags.ParamTag was susceptible to problems in containers that pool tags. It wasn't resetting values when the release method was called, so if a user used multiple spring:param tags and mixed using the value attribute and tag body, it's possible that the value used might not be correct. This would be very difficult to duplicate. I just noticed it while making the similar ArgumentTag and decided it should be fixed while I was in there. I also added two additional unit tests to test this behavior change. Issue: SPR-9678
|
I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement. |
|
Has anyone had a chance to look at this simply change? I'd love to get this in before 4.0.M2. |
|
*simple change |
|
I posted this pull request nearly six weeks ago. I understand that some back-and-forth and revisions may be necessary before it can be approved. However, is six weeks I haven't received a single comment. The back-and-forth and revisions can't happen if I don't get any feedback. Can someone please look at this? I don't want it to sit there for so long that it's no longer mergable, and I really wanted it to be merged for Milestone 2. |
|
I assume from the lack of comments after nearly seven weeks that this pull request is perfect :) ... can someone please merge it so that it will be part of Milestone 2? |
|
I started looking at this yesterday, from what I can tell it pretty much is perfect :) M2 has been pushed back a couple of days so that we can get to a few issues like this one. The one change I was thinking of making was to allow the argument tags and the comma values to be used together, rather than either/or. |
|
Thanks! As for your thoughts on allowing argument tags and comma values to be used together, I don't see how that can work. Unlike the <spring:message code="my.message.code" arguments="15.25,World">
<spring:argument value="8.6" />
<spring:argument>This is another argument</spring:argument>
</spring:message>Which argument is argument number 1? 2? 3? 4? IMO, this doesn't work and is extremely confusing. Requiring either/or is the simplest approach and doesn't require the user to understand the rules of merging the |
# By Nicholas Williams * SPR-9678: Add <spring:argument> subtag for message/theme Ensure ParamTag release resources Fix malformed UrlTag Javadoc Update TLD versions to 4.0
|
I see your point but I have run with the merge arguments options for now. I don't think it's too confusing since the arguments attribute always appears before the nested tags so the order seems fairly logical to me. I also wanted to get this in before M2, we can refine before RC1 if you feel strongly that this is the wrong call. |
|
I do believe that could cause more problems than it will solve, but we'll see. Thanks for merging! :) |
Added a new
<spring:argument>tag meant for nesting within<spring:message>and<spring:theme>. The tag is based on the<fmt:param>tag and uses conventions found throughout other Spring tags.Ten new unit tests and over a dozen changed unit tests accompany the
changes, which are detailed below.
Itemized Changes:
<spring:argument>tag tospring.tld.spring.tld.o.s.web.servlet.tags.ArgumentTagclass to handle tag. Added 5unit tests to test behavior.
o.s.web.servlet.tags.ArgumentAwareto mark tags supportingarguments.
o.s.web.servlet.tags.MessageTagto implementArgumentAwareand permit either nested argument tags or the
argumentsattribute, butnot both. Also updated JavaDoc. Added three unit tests to test behavior.
Updated over a dozen other unit tests to test behavior.
o.s.web.servlet.tags.ThemeTagJavaDoc.@linkerror in the JavaDoc foro.s.web.servlet.tags.UrlTago.s.web.servlet.tags.ParamTagwas susceptible to problems incontainers that pool tags. It wasn't resetting values when the
release method was called, so if a user used multiple
<spring:param>tags and mixed using the
valueattribute and tag body, it's possiblethat the value used might not be correct. This would be very
difficult to duplicate. I just noticed it while making the similar
ArgumentTagand decided it should be fixed while I was in there.I also added two additional unit tests to test this behavior change.
Issue: SPR-9678