On Mon, 2011-10-31 at 13:07 +0100, Francesco Chicchiriccò wrote:
> On 29/10/2011 18:37, Thorsten Scherler wrote:
> > On Sat, 2011-10-29 at 16:39 +0200, Francesco Chicchiriccò wrote:
> >> On 28/10/2011 12:58, Thorsten Scherler wrote:
> >>> [...]
> >> Finally, I've also made a fix for passing non-String parameters to ST,
> >> so $if$ is actually doing its job (take a look at cocoon-stringtemplate
> >> unit tests): unfortunately, this does not seem to work for sitemap, so I
> >> preferred not to update StringTemplate samples in cocoon-sample.
> >>
> >> Can anyone confirm (and possibly point out where to look, in case) that
> >> sitemap parameters are always cast to String?
> > /cocoon-sitemap/src/main/java/org/apache/cocoon/sitemap/InvocationImpl.java
> >
> > resolveParameter(String){
> > ...
> >   return result.toString();
> > }
> >
> > I am not sure if we can change
> > org.apache.cocoon.sitemap.node.SitemapNode but in that interface we
> > define:
> >   void setParameters(Map<String, String>  parameters);
> > I reckon Map<String, Object>  would the one we are looking for.
> 
> Hi,
> see attached a patch letting sitemap parameters be actual Objects: check 
> updated StringTemplate samples in cocoon-sample in order to see 
> effective $if$ (finally!) in place.
> 
> For the moment I preferred not to commit the attached patch, because it 
> is a quite intrusive change (talking about interfaces, not only 
> implementations).
> Besides all unit and integration tests defined, I've also made some 
> others by-hand testing, and it *should* work.
> 
> Please let me know if you run into any problem and if you think this 
> patch to be committed.

I had a quick look and it seems fine with me. The only problem I had
with the path is that it introduced some formating changes as well -
making it hard to see right away the important change. 

I try to prevent this situation in 
a) format the classes you will need to change BEFORE any change
b) commit that as "whitenoise"
c) do the work

This way only the important changes can be seen on the patch without any
"whitenoise".

I would be +1.

Maybe you want to call a vote to catch attention?

salu2
-- 
Thorsten Scherler <thorsten.at.apache.org>
codeBusters S.L. - web based systems
<consulting, training and solutions>
http://www.codebusters.es/

Reply via email to