[
https://issues.apache.org/struts/browse/WW-2807?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=44731#action_44731
]
Christopher Schultz commented on WW-2807:
-----------------------------------------
After doing a bit of poking around in the code, I think this might actually be
a trivial patch. It looks like every tag (a Directive in Velocity-speak)
inherits from AbstractDirective whose createPropertyMap method appears to be
responsible for parsing the parameters and creating ... a Map!
This method could be changed to check for a single Map parameter and simple
return that with no further processing:
for (int index = 0, length = children; index < length; index++) {
this.putProperty(propertyMap, contextAdapter,
node.jjtGetChild(index));
}
Becomes:
if(node.jjtGetChild(0) instanceof Map)
return node.jjtGetChild(0);
for (int index = 0, length = children; index < length; index++) {
this.putProperty(propertyMap, contextAdapter,
node.jjtGetChild(index));
}
(minus null checking, etc.)
I would appreciate some feedback, here, because I don't understand the nuances
of the expression language usage when coupled with Velocity (which pretty much
parses everything beforehand, anyway). We might want to do something like what
putProperty does:
// node.value uses the StrutsValueStack to evaluate the directive's
value parameter
String param = node.value(contextAdapter).toString();
Let me know what makes sense, here.
Thanks!
> Improve Velocity tag syntax and performance
> -------------------------------------------
>
> Key: WW-2807
> URL: https://issues.apache.org/struts/browse/WW-2807
> Project: Struts 2
> Issue Type: Improvement
> Affects Versions: 2.0.0, 2.0.1, 2.0.2, 2.0.3, 2.0.4, 2.0.5, 2.0.6, 2.0.7,
> 2.0.8, 2.0.9, 2.0.10, 2.0.11, 2.0.11.1, 2.0.11.2, 2.0.12, 2.1.0, 2.1.1,
> 2.1.2, 2.1.3, 2.1.4, 2.2.x, Future
> Environment: Apache Velocity 1.5+
> Reporter: Christopher Schultz
> Priority: Minor
>
> Currently, Velocity "tags" are implemented by passing a number of
> "name=value" strings to S2's Velocity tag implementation in order to pass
> named parameters (which are not directly supported by Velocity). This leads
> to a somewhat awkward syntax and requires that the tag implementation
> re-parse these strings for every template evaluation, loop execution, etc.
> Starting with Velocity 1.5, template authors can create Map objects on the
> fly using a simple syntax. I believe that supporting this syntax in the
> Velocity S2 integration code will make templates easier to read and run more
> quickly.
> Example of current Velocity tag usage (this is the example from
> http://struts.apache.org/2.0.11.2/docs/velocity-tags.html):
> #sform ("action=updatePerson")
> #stextfield ("label=First name" "name=firstName")
> #ssubmit ("value=Update")
> #end
> The new, proposed syntax would be this:
> #sform({'action' : 'updatePerson' })
> #stextfield({'label' : 'First name', 'name' : 'firstName'})
> #ssubmit({'value' = 'Update'})
> #end
> The tag implementation has change from supporting an unknown number of
> unnamed string parameters to accepting a single Map parameter containing
> name/value pairs. S2's code will no longer hand to parse each of the string
> parameters into their separate name and value, and should therefore run
> faster.
> It also allows template designers to use single-quote syntax for their
> parameter names (single-quoted strings are never interpreted) and either
> double-quote syntax for their values (which /will/ interpret their contents)
> or pass the objects directly (possibly as non-String objects). I believe this
> will also increase performance as well as flexibility, since some tags will
> prefer non-String parameters and will no longer be required to parse a String
> unless absolutely necessary.
> Such a change would require one of the following strategies:
> 1. Implement a second Velocity tag library whose tags accept a single Map
> parameter
> 2. Augment the current Velocity tag library to accept /either/ a single Map
> parameter /or/ a series of String values
> I believe #2 would be a better solution: users can choose which syntax they
> prefer and don't have to go through any messy re-configuration steps to
> switch syntax. It also supports legacy users with no changes at all. The only
> downside is that you'd have to restrict any future tags such that they do not
> accept a single Map parameter and then behave differently. I think this is a
> reasonable trade-off.
> I would be happy to submit a sample implementation for one or two tags, but
> I'll probably need help doing /every/ tag.
> Thanks,
> -chris
> Please see the following mailing list threads for reference:
> [struts-dev]
> http://www.nabble.com/-s2--Improved-Velocity-Integration-td19541213.html
> [struts-user] http://www.nabble.com/-S2--Velocity-Integration-td19520425.html
> [velocity-dev] http://www.nabble.com/Named-macro-parameters-td19459070.html
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
