Add <spring:argument> subtag for <spring:message> [SPR-9678]

[spring-projects-issues]

Nick Williams opened SPR-9678 and commented

The JSTL tag <c:url> can contain parameter placeholders in its value, and nested <c:param> tags supply values for those placeholders. Likewise the spring:url tag can contain parameter placeholders in its value, and nested spring:param tags supply values for those placeholders.

However, things differ for i18n. fmt:message, like <c:url>, uses nested fmt:param tags to replace message parameters. However, spring:message does not. It instead has a flat attribute that expects a comma-(by default)-separated LIST of arguments, and contains an optional attribute for specifying a different delimiter.

This is an onerous way to use this feature. If I have a parameter in a message that can contain any number of typical delimiters (for example, I'm displaying an error message), I have to go to extreme lengths to display this properly without worrying about improper splitting:

<spring:message code="error.logged.message" argumentSeparator=";||?||;" arguments="${exception.localizedMessage}" />

This would make much more sense, and match not only the JSTL but also other Spring tags (like spring:url):

<spring:message code="error.logged.message"><spring:param value="${exception.localizedMessage}" /></spring:message>

I believe this is a much-needed improvement for this tag, and the existing arguments and argumentSeparator attributes could be left in place for backwards compatibility.

Would love to see this in 3.2.x... :-)

---

Affects: 3.0.1, 3.0.7, 3.1.1, 3.1.2

Referenced from: commits 66cfe9f, f9b17a7

1 votes, 5 watchers

[spring-projects-issues]

Nick Williams commented

So, you can take what I said about c:url/spring:url with a grain of salt, because their param subtags do something slightly different. But the concept is basically the same, and using spring:message is still difficult compared to fmt:message.

[spring-projects-issues]

Jarosław Kątnik commented

I'm missing this too.

[spring-projects-issues]

Nick Williams commented

Can I get some comment from the Spring team? This seems like such a glaring thing missing from the <spring:message> tag. Additionally, after looking through the code it looks like it's as simple as org.springframework.web.servlet.tags.MessageTag implementing org.springframework.web.servlet.tags.ParamAware to achieve this. Maybe at least in Spring 4.0.M2 since it's too late for Spring 3.2?

[spring-projects-issues]

Nick Williams commented

In addition to making MessageTag implement ParamAware, you also need to change the spring.tld file to make the name attribute of the <spring:param> tag not required (change <required>true</required> to <required>false</required>). Then you'll need to update the org.springframework.web.servlet.tags.UrlTag's addParam method to validate that the name is non-null and throw an exception otherwise.

The addParam method for MessageTag would, theoretically, throw an exception if arguments had already been set using the arguments attribute.

Finally, if you don't want to re-use the <spring:param> tag for this (which I understand, what with it having the name attribute that is unnecessary and potentially confusing here), I would suggest simply making Argument, ArgumentTag, and ArgumentAware classes/interface and creating a <spring:argument> tag instead. This would also be a pretty trivial task.

[spring-projects-issues]

Nick Williams commented

Now that I think about it, I think <spring:argument> is the better option here. If someone will confirm that my understanding of the fork/branch/change/pull procedure is correct, I can get started:

1. Fork SpringSource/spring-framework
2. Make a branch from master in my fork called Add <spring:argument> subtag for <spring:message> [SPR-9678] #14312 (the ID of this bug)
3. Make the change in the branch
4. Submit a pull request from my Add <spring:argument> subtag for <spring:message> [SPR-9678] #14312 branch to the SpringSource/spring-framework master branch

[spring-projects-issues]

Nick Williams commented

I have submitted pull request 297 with the new <spring:argument> tag implemented. Provided it's accepted, it should close this issues. I verified the changes with many unit tests and lots of functional testing in my target appliction.

I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement.

[spring-projects-issues]

Rossen Stoyanchev commented

Thanks for the pull request. Will have a look.

[spring-projects-issues]

Nick Williams commented

I submitted this pull request three weeks ago now. I was hoping to get it into Milestone 2, which releases Monday. Could somebody review this pull request so that I can make any changes, if necessary, and get it merged? Thanks!

[spring-projects-issues]

Nick Williams commented

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.

[spring-projects-issues]

Nick Williams commented

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?

[spring-projects-issues]

Nick Williams commented

Thanks!