[
https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12538881
]
Nathan Bubna commented on VELTOOLS-88:
--------------------------------------
Nice feature. It sorta overlaps the self() method when selfIncludeParameters
is changed to true, but as the default for that is false, this is a cleaner way
to add them than doing $link.params($request.parameterMap), especially with the
ability to ignore certain params. That said, here's some critiques before we
put this in:
- The self() method will need to respect the paramsToIgnore when it is
configured to include params.
- I'm not thrilled about using addQueryData(name,value) for each individual
parameter. If there's a lot of params, that will needlessly create and discard
a lot of LinkTool clones. I think it would be better if it copied the
request.getParameterMap(), removed any params to be ignored from that and then
passed the result to params(Map).
- The javadoc references ignore(String) which is not in the patch.
- addIgnore() should return a copy, not this. Which means you'll probably need
to add a new copyWithIgnore(String) method and make sure that the
paramsToIgnore are always copied to new copies.
Sorry if that seems like a lot, but i'd like to avoid introducing
inconsistencies in how LinkTool behaves. speaking of which, i just noticed a
few things in it that need attention. :)
let me know if you have questions about my feedback. i'd be happy to help, as
i think this is a nice new feature.
> LinkTool could easily be modified to support current-request parameter
> duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the
> StrutsLinkTool that added two methods: ignore(String paramName) and
> addAllParameters(). Together, they could be used to copy all the parameters
> from the current request into the link currently being built, and you could
> ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to
> myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool
> has the same capability. Since that team decided to add this feature, I'm
> offering to add it to the LinkTool, which will provide the same capability to
> all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would
> merely be a new method which supports the new feature. Its use is entirely
> optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request
> parameter explicitly set in the link builder, while my implementation
> requires that you explicitly ignore those parameters. My enhancement can
> follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
