You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Nathan Bubna <nb...@gmail.com> on 2007/11/09 21:15:28 UTC
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
hey Claude, i'm not sure if you're done with this or if it's still in
process, but there's a few problems so far (in decreasing order of
importance):
- ant test is now failing
- the hasSubKeys things doesn't make any sense to me
- stylistically, the practice for Velocity projects is to keep braces
on their own lines
i'm also curious about what the advantages are of this approach (as
opposed to a ValueParserSub). it's been a while since i've thought
through the implementation options for this.
On Nov 9, 2007 7:15 AM, Claude Brisson <cl...@renegat.net> wrote:
> ValueParser now has protected get/setAllowSubkeys() boolean methods.
>
> The default value of allowSubkey shoud be the value of deprecatedMode,
> but I'm not really sure of how this should be done. But I'm sure Nathan
> will be of some help here.
>
> Also, since ValueParser is used internally by the Tool.configure method,
> it may be safer to move all this new stuff in ParameterParser only.
>
> Claude
>
>
> Le vendredi 09 novembre 2007 à 14:49 +0000, cbrisson@apache.org a
> écrit :
>
> > Author: cbrisson
> > Date: Fri Nov 9 06:49:36 2007
> > New Revision: 593549
> >
> > URL: http://svn.apache.org/viewvc?rev=593549&view=rev
> > Log:
> > sub keys getter (not yet tested but doesnt break anything at least)
> >
> > Modified:
> > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> >
> > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java?rev=593549&r1=593548&r2=593549&view=diff
> > ==============================================================================
> > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java (original)
> > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java Fri Nov 9 06:49:36 2007
> > @@ -21,6 +21,9 @@
> >
> > import java.util.Map;
> > import java.util.Locale;
> > +import java.util.Set;
> > +import java.util.HashMap;
> > +
> > import org.apache.velocity.tools.config.DefaultKey;
> >
> > /**
> > @@ -40,6 +43,14 @@
> > {
> > private Map source = null;
> >
> > + private boolean allowSubkeys = true; /* default to whatever, should be overridden by depreprecatedMode default value anyway */
> > +
> > + /* when using subkeys, cache at least the presence of any subkey,
> > + so that the rendering of templates not using subkeys will only
> > + look once for subkeys
> > + */
> > + private boolean hasSubkeys = true;
> > +
> > public ValueParser() {}
> >
> > public ValueParser(Map source)
> > @@ -98,7 +109,11 @@
> > {
> > return null;
> > }
> > - return getSource().get(key);
> > + Object value = getSource().get(key);
> > + if (value == null && getAllowSubkeys()) {
> > + value = getSubkey(key);
> > + }
> > + return value;
> > }
> >
> > public Object[] getValues(String key)
> > @@ -359,4 +374,29 @@
> > return toLocales(getValues(key));
> > }
> >
> > + protected boolean getAllowSubkeys() {
> > + return allowSubkeys;
> > + }
> > +
> > + protected void setAllowSubkeys(boolean allow) {
> > + allowSubkeys = allow;
> > + }
> > +
> > + protected Object getSubkey(String subkey) {
> > + if (!hasSubkeys || subkey == null || subkey.length() == 0) {
> > + return null;
> > + }
> > + Map<String,Object> values = null;
> > + subkey = subkey.concat(".");
> > + for(Map.Entry<String,Object> entry:(Set<Map.Entry>)getSource().entrySet()) {
> > + if(entry.getKey().startsWith(subkey)) {
> > + if(values == null) {
> > + values = new HashMap<String,Object>();
> > + }
> > + values.put(entry.getKey().substring(subkey.length()),entry.getValue());
> > + }
> > + }
> > + hasSubkeys = (values == null);
> > + return values;
> > + }
> > }
> >
> > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java?rev=593549&r1=593548&r2=593549&view=diff
> > ==============================================================================
> > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java (original)
> > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java Fri Nov 9 06:49:36 2007
> > @@ -109,7 +109,11 @@
> > */
> > public Object getValue(String key)
> > {
> > - return getRequest().getParameter(key);
> > + Object value = getRequest().getParameter(key);
> > + if(value == null && getAllowSubkeys()) {
> > + value = getSubkey(key);
> > + }
> > + return value;
> > }
> >
> >
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
Posted by Nathan Bubna <nb...@gmail.com>.
On Nov 13, 2007 8:52 AM, Nathan Bubna <nb...@gmail.com> wrote:
> On Nov 13, 2007 8:07 AM, Nathan Bubna <nb...@gmail.com> wrote:
> > On Nov 13, 2007 3:46 AM, Claude Brisson <cl...@renegat.net> wrote:
> > > Le lundi 12 novembre 2007 à 16:18 -0800, Nathan Bubna a écrit :
> <snip/>
>
> > > > > My feeling here is that although foo.int is a cool syntax, it has too
> > > > > many backwards ; especially, we should always return the integral type
> > > > > (string, boolean or number) when available rather than a wrapper around
> > > > > it to avoid nasty side effects.
> > > >
> > > > c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
> > > > even a returning a simple HashMap would be returning a wrapper. as
> > > > long as we make the subkey business configurable (with it off when in
> > > > deprecation support mode), and only return the
> > > > ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
> > > > that start with "foo.", then i think we'll be fine.
> > >
> > > You don't get my point.
> >
> > i would say the same... :)
> >
> > > By construction, an application should usually
> > > know when expecting a map or an integral type. Let's consider those
> > > points separately:
> > >
> > > - maps: both implementations do wrap them. I guess both are valid but I
> > > prefer the new one because I don't see why we should use two different
> > > wrappers (ValueParser itself is already a wrapper around a map). Having
> > > the wrapper extend Map respects the principle of least surprise for
> > > template coders.
> >
> > that helps only for Maps, which i suspect will not often be found
> > inside the source map.
> >
> > > - integral types: wrapping them is not a problem as long as they're
> > > used directly for display (and it allows the foo.int syntax), but
> > > whenever those values will be used as arguments to other tools or
> > > objects methods you will encounter side effects (and don't even think of
> > > trying to detect the appropriate conversion by reflection, it only works
> > > for very simple cases). $list.add($params.foo) would add the
> > > ValueParserSub to the list, not the integral type... and there are
> > > plenty of examples like this one. In fact, specifying the type
> > > (.int, .string, ...) becomes mandatory to get rid of the wrapper. Which
> > > is rather cumbersome.
> >
> > and what gets added to the $list in your current implementation when
> > subkeys are allowed and there is also a "foo.bar" available in the
> > source map? $list.add() receives a ValueParser instead of the
> > integral type, right? so once again, it becomes necessary to get rid
> > of the wrapper. but how? the new ValueParser returned by $params.foo
> > provides no means to unwrap itself, cumbersome or otherwise. if you
> > really just want the "foo" parameter and not a "foo.bar" one, then it
> > seems you are out of luck.
>
> i just re-examined the current code. i wasn't wrong. in your current
> implementation, $params.foo *will* return the integral type for "foo"
> if there is one. However, the presence of "foo" means that
> $params.foo.bar syntax will fail. This unsettles me somewhat as it
> seems rather fragile and unpredictable. Granted, to do things as i
> had anticipated (have $params.foo return a wrapper anytime a "foo.bar"
> type parameter is available), makes the type returned by $params.foo
> unpredictable, but this seems less troublesome than making it
> unpredictable whether anything at all will be returned.
>
> i've got to get some other work done now, but i'll be thinking about
> this more...
ok, after much thought, i think i prefer the better backwards
compatibility of always having $params.foo return its string value to
returning a ValueParser or ValueParserSub when subkeys for "foo" are
available and a string when there are no "foo" subkeys. the better
backwards compatibility is slightly more important to me than having
the subkey syntax work consistently. we will, however, need to be
sure and document clearly that the subkey syntax is unreliable in such
situations.
of course, i could be swayed the other way on this in the future if
this starts to pose problems, but for now, i'm onboard with the
current implementation. thanks for bearing with my obstinance and
inattention... :)
> > > Re-introducing the foo.int syntax would be cool, of course, but why not
> > > try to do it in the engine itself (so it is generalized to every value)?
> > > It'd be cool to be able to do "$foo.int" on every value (once one have
> > > checked that "int" is not a valid key for "foo").
> >
> > that is a very interesting idea. it won't solve the problem i
> > describe above, but it could be cool.
> >
> >
> > >
> > > Claude
> > >
> > > > <snip/>
> > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > > For additional commands, e-mail: dev-help@velocity.apache.org
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > For additional commands, e-mail: dev-help@velocity.apache.org
> > >
> > >
> >
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
Posted by Nathan Bubna <nb...@gmail.com>.
On Nov 13, 2007 8:07 AM, Nathan Bubna <nb...@gmail.com> wrote:
> On Nov 13, 2007 3:46 AM, Claude Brisson <cl...@renegat.net> wrote:
> > Le lundi 12 novembre 2007 à 16:18 -0800, Nathan Bubna a écrit :
<snip/>
> > > > My feeling here is that although foo.int is a cool syntax, it has too
> > > > many backwards ; especially, we should always return the integral type
> > > > (string, boolean or number) when available rather than a wrapper around
> > > > it to avoid nasty side effects.
> > >
> > > c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
> > > even a returning a simple HashMap would be returning a wrapper. as
> > > long as we make the subkey business configurable (with it off when in
> > > deprecation support mode), and only return the
> > > ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
> > > that start with "foo.", then i think we'll be fine.
> >
> > You don't get my point.
>
> i would say the same... :)
>
> > By construction, an application should usually
> > know when expecting a map or an integral type. Let's consider those
> > points separately:
> >
> > - maps: both implementations do wrap them. I guess both are valid but I
> > prefer the new one because I don't see why we should use two different
> > wrappers (ValueParser itself is already a wrapper around a map). Having
> > the wrapper extend Map respects the principle of least surprise for
> > template coders.
>
> that helps only for Maps, which i suspect will not often be found
> inside the source map.
>
> > - integral types: wrapping them is not a problem as long as they're
> > used directly for display (and it allows the foo.int syntax), but
> > whenever those values will be used as arguments to other tools or
> > objects methods you will encounter side effects (and don't even think of
> > trying to detect the appropriate conversion by reflection, it only works
> > for very simple cases). $list.add($params.foo) would add the
> > ValueParserSub to the list, not the integral type... and there are
> > plenty of examples like this one. In fact, specifying the type
> > (.int, .string, ...) becomes mandatory to get rid of the wrapper. Which
> > is rather cumbersome.
>
> and what gets added to the $list in your current implementation when
> subkeys are allowed and there is also a "foo.bar" available in the
> source map? $list.add() receives a ValueParser instead of the
> integral type, right? so once again, it becomes necessary to get rid
> of the wrapper. but how? the new ValueParser returned by $params.foo
> provides no means to unwrap itself, cumbersome or otherwise. if you
> really just want the "foo" parameter and not a "foo.bar" one, then it
> seems you are out of luck.
i just re-examined the current code. i wasn't wrong. in your current
implementation, $params.foo *will* return the integral type for "foo"
if there is one. However, the presence of "foo" means that
$params.foo.bar syntax will fail. This unsettles me somewhat as it
seems rather fragile and unpredictable. Granted, to do things as i
had anticipated (have $params.foo return a wrapper anytime a "foo.bar"
type parameter is available), makes the type returned by $params.foo
unpredictable, but this seems less troublesome than making it
unpredictable whether anything at all will be returned.
i've got to get some other work done now, but i'll be thinking about
this more...
> > Re-introducing the foo.int syntax would be cool, of course, but why not
> > try to do it in the engine itself (so it is generalized to every value)?
> > It'd be cool to be able to do "$foo.int" on every value (once one have
> > checked that "int" is not a valid key for "foo").
>
> that is a very interesting idea. it won't solve the problem i
> describe above, but it could be cool.
>
>
> >
> > Claude
> >
> > > <snip/>
> >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > For additional commands, e-mail: dev-help@velocity.apache.org
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > For additional commands, e-mail: dev-help@velocity.apache.org
> >
> >
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
Posted by Nathan Bubna <nb...@gmail.com>.
On Nov 13, 2007 3:46 AM, Claude Brisson <cl...@renegat.net> wrote:
> Le lundi 12 novembre 2007 à 16:18 -0800, Nathan Bubna a écrit :
> > ah. ok, looked closer at the latest version. looks good. thx. :)
> > but now i have a new question... why the expandSingletons stuff? we
> > don't expand them for $params.foo, why should we expand them for
> > $params.foo.bar?
> >
> > i'm assuming it's because request params automatically come as arrays
> > when you iterate over the parameter map. but i would like
> > ValueParser to be more than just the base for ParameterTool, and it
> > doesn't make sense for ValueParser.
>
> Yes, I'll move the expansion stuff to ParameterTool, that's cleaner.
> Also, we may reflect this expansion in all methods of the Map interface
> for ParameterTool.
thanks!
> > > Btw, I hoped that by having ValueParser implement Map I'd see
> > > ValueParser objects displayed like "{ key=value ... }" but I still see
> > > the ugly org.apache.velocity.tools.generic.ValueParser@848ecc, I'll dig
> > > into this...
> >
> > nothing special about implementing the Map interface when it comes to
> > rendering. Velocity simply renders stuff by calling toString().
> > That's all that needs to change to change the display.
>
> Extending the Map interface was also motivated to allow cool thinks like
> $map.putAll($params)
very nice.
> I'll code us a nice toString().
>
> > > > > The proposed implementation only returns a new ValueParser object
> > > > > whenever subkeys are allowed and found.
> > > >
> > > > same could be done with a Sub. granted, the implementation would have
> > > > to be smarter (i.e. search for a potential matching subkeys first)
> > > > than the last implementation for this, but there's no reason it can't
> > > > be done.
> > >
> > > Sure, but it all boils down to the decision to keep the foo.int syntax
> > > or not. If yes, then we definitely need a ValueParserSub object, if no,
> > > then returning either a new ValueParser (when subkeys are found) or the
> > > value in its integral type looks more natural to me.
> >
> > yeah. seems like just a question of whether we want the foo.int
> > syntax or not. since it doesn't seem to be your itch, don't worry
> > about it. if i get around to it, i'll do it. if not, no big deal.
> >
> > > My feeling here is that although foo.int is a cool syntax, it has too
> > > many backwards ; especially, we should always return the integral type
> > > (string, boolean or number) when available rather than a wrapper around
> > > it to avoid nasty side effects.
> >
> > c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
> > even a returning a simple HashMap would be returning a wrapper. as
> > long as we make the subkey business configurable (with it off when in
> > deprecation support mode), and only return the
> > ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
> > that start with "foo.", then i think we'll be fine.
>
> You don't get my point.
i would say the same... :)
> By construction, an application should usually
> know when expecting a map or an integral type. Let's consider those
> points separately:
>
> - maps: both implementations do wrap them. I guess both are valid but I
> prefer the new one because I don't see why we should use two different
> wrappers (ValueParser itself is already a wrapper around a map). Having
> the wrapper extend Map respects the principle of least surprise for
> template coders.
that helps only for Maps, which i suspect will not often be found
inside the source map.
> - integral types: wrapping them is not a problem as long as they're
> used directly for display (and it allows the foo.int syntax), but
> whenever those values will be used as arguments to other tools or
> objects methods you will encounter side effects (and don't even think of
> trying to detect the appropriate conversion by reflection, it only works
> for very simple cases). $list.add($params.foo) would add the
> ValueParserSub to the list, not the integral type... and there are
> plenty of examples like this one. In fact, specifying the type
> (.int, .string, ...) becomes mandatory to get rid of the wrapper. Which
> is rather cumbersome.
and what gets added to the $list in your current implementation when
subkeys are allowed and there is also a "foo.bar" available in the
source map? $list.add() receives a ValueParser instead of the
integral type, right? so once again, it becomes necessary to get rid
of the wrapper. but how? the new ValueParser returned by $params.foo
provides no means to unwrap itself, cumbersome or otherwise. if you
really just want the "foo" parameter and not a "foo.bar" one, then it
seems you are out of luck.
> Re-introducing the foo.int syntax would be cool, of course, but why not
> try to do it in the engine itself (so it is generalized to every value)?
> It'd be cool to be able to do "$foo.int" on every value (once one have
> checked that "int" is not a valid key for "foo").
that is a very interesting idea. it won't solve the problem i
describe above, but it could be cool.
>
> Claude
>
> > <snip/>
>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > For additional commands, e-mail: dev-help@velocity.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in
/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools:
generic/ValueParser.java view/ParameterTool.java
Posted by Claude Brisson <cl...@renegat.net>.
Le lundi 12 novembre 2007 à 16:18 -0800, Nathan Bubna a écrit :
> ah. ok, looked closer at the latest version. looks good. thx. :)
> but now i have a new question... why the expandSingletons stuff? we
> don't expand them for $params.foo, why should we expand them for
> $params.foo.bar?
>
> i'm assuming it's because request params automatically come as arrays
> when you iterate over the parameter map. but i would like
> ValueParser to be more than just the base for ParameterTool, and it
> doesn't make sense for ValueParser.
Yes, I'll move the expansion stuff to ParameterTool, that's cleaner.
Also, we may reflect this expansion in all methods of the Map interface
for ParameterTool.
> > Btw, I hoped that by having ValueParser implement Map I'd see
> > ValueParser objects displayed like "{ key=value ... }" but I still see
> > the ugly org.apache.velocity.tools.generic.ValueParser@848ecc, I'll dig
> > into this...
>
> nothing special about implementing the Map interface when it comes to
> rendering. Velocity simply renders stuff by calling toString().
> That's all that needs to change to change the display.
Extending the Map interface was also motivated to allow cool thinks like
$map.putAll($params)
I'll code us a nice toString().
> > > > The proposed implementation only returns a new ValueParser object
> > > > whenever subkeys are allowed and found.
> > >
> > > same could be done with a Sub. granted, the implementation would have
> > > to be smarter (i.e. search for a potential matching subkeys first)
> > > than the last implementation for this, but there's no reason it can't
> > > be done.
> >
> > Sure, but it all boils down to the decision to keep the foo.int syntax
> > or not. If yes, then we definitely need a ValueParserSub object, if no,
> > then returning either a new ValueParser (when subkeys are found) or the
> > value in its integral type looks more natural to me.
>
> yeah. seems like just a question of whether we want the foo.int
> syntax or not. since it doesn't seem to be your itch, don't worry
> about it. if i get around to it, i'll do it. if not, no big deal.
>
> > My feeling here is that although foo.int is a cool syntax, it has too
> > many backwards ; especially, we should always return the integral type
> > (string, boolean or number) when available rather than a wrapper around
> > it to avoid nasty side effects.
>
> c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
> even a returning a simple HashMap would be returning a wrapper. as
> long as we make the subkey business configurable (with it off when in
> deprecation support mode), and only return the
> ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
> that start with "foo.", then i think we'll be fine.
You don't get my point. By construction, an application should usually
know when expecting a map or an integral type. Let's consider those
points separately:
- maps: both implementations do wrap them. I guess both are valid but I
prefer the new one because I don't see why we should use two different
wrappers (ValueParser itself is already a wrapper around a map). Having
the wrapper extend Map respects the principle of least surprise for
template coders.
- integral types: wrapping them is not a problem as long as they're
used directly for display (and it allows the foo.int syntax), but
whenever those values will be used as arguments to other tools or
objects methods you will encounter side effects (and don't even think of
trying to detect the appropriate conversion by reflection, it only works
for very simple cases). $list.add($params.foo) would add the
ValueParserSub to the list, not the integral type... and there are
plenty of examples like this one. In fact, specifying the type
(.int, .string, ...) becomes mandatory to get rid of the wrapper. Which
is rather cumbersome.
Re-introducing the foo.int syntax would be cool, of course, but why not
try to do it in the engine itself (so it is generalized to every value)?
It'd be cool to be able to do "$foo.int" on every value (once one have
checked that "int" is not a valid key for "foo").
Claude
> <snip/>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
Posted by Nathan Bubna <nb...@gmail.com>.
On Nov 12, 2007 3:13 PM, Claude Brisson <cl...@renegat.net> wrote:
> Le lundi 12 novembre 2007 à 10:53 -0800, Nathan Bubna a écrit :
> > it's not the name so much as how it's used. it looks to me like once
> > it searches for subkey "foo", it won't even try to find subkey "bar"
> > or "woogie" or whatever. either that's wrong or i'm missing
> > something?
>
> The "hasSubkeys" boolean is here so that we don't search for subkeys
> twice if we already know there isn't any (so that the overhead when
> looking for inexisting keys, in templates that don't use subkeys, is
> reduced to only one indeOf('.') in all key names). The very first
> implementation was buggy, it may be the one you read.
ah. ok, looked closer at the latest version. looks good. thx. :)
but now i have a new question... why the expandSingletons stuff? we
don't expand them for $params.foo, why should we expand them for
$params.foo.bar?
i'm assuming it's because request params automatically come as arrays
when you iterate over the parameter map. but i would like
ValueParser to be more than just the base for ParameterTool, and it
doesn't make sense for ValueParser.
> > > > i'm also curious about what the advantages are of this approach (as
> > > > opposed to a ValueParserSub). it's been a while since i've thought
> > > > through the implementation options for this.
> > >
> > > Things have changed since then: the engine is now quite efficient in
> > > automatic types conversions so the "foo.int" syntax is much less
> > > necessary than "foo.bar", and you always have "foo.getInt()".
> >
> > huh? foo.int is the same as foo.getInt(). you can't have the latter
> > if the former doesn't work. did you mean getInt('foo')?
>
> Yes, sorry.
no prob. just want to make sure i understand.
> > also, the
> > sub could handle both foo.bar and foo.int, and the engine can only do
> > a small fraction of the conversions the ValueParser is capable of.
> > so, while i don't see anything wrong with the current approach (apart
> > from the hasSubKey thing), i still suspect a Sub has more potential,
> > since we have complete control of the API.
> >
> > > But above all, having the generic getter return something else (that is,
> > > the ValueParserSub) than the underlying basic type leads - at least in
> > > my experience - to several side effects (like a jdbc driver being unable
> > > to handle the ValueParserSub class by calling toString on it).
> >
> > not sure how returning ValueParserSub is that different from returning
> > ValueParser.
>
> We only return a ValueParser for get("foo") when there are "foo.bar"
> keys, and keep returning the integral type otherwise. Since we expect
> the same fonctionnalities for $params and $params.foo, the recursive
> approach looked simpler to me.
perhaps from the standpoint of developing the tool itself, but neither
$params.getInt('foo.bar') nor $params.foo.getInt('bar') is as simple
to a user as $params.foo.bar.int
> Btw, I hoped that by having ValueParser implement Map I'd see
> ValueParser objects displayed like "{ key=value ... }" but I still see
> the ugly org.apache.velocity.tools.generic.ValueParser@848ecc, I'll dig
> into this...
nothing special about implementing the Map interface when it comes to
rendering. Velocity simply renders stuff by calling toString().
That's all that needs to change to change the display.
> > > The proposed implementation only returns a new ValueParser object
> > > whenever subkeys are allowed and found.
> >
> > same could be done with a Sub. granted, the implementation would have
> > to be smarter (i.e. search for a potential matching subkeys first)
> > than the last implementation for this, but there's no reason it can't
> > be done.
>
> Sure, but it all boils down to the decision to keep the foo.int syntax
> or not. If yes, then we definitely need a ValueParserSub object, if no,
> then returning either a new ValueParser (when subkeys are found) or the
> value in its integral type looks more natural to me.
yeah. seems like just a question of whether we want the foo.int
syntax or not. since it doesn't seem to be your itch, don't worry
about it. if i get around to it, i'll do it. if not, no big deal.
> My feeling here is that although foo.int is a cool syntax, it has too
> many backwards ; especially, we should always return the integral type
> (string, boolean or number) when available rather than a wrapper around
> it to avoid nasty side effects.
c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
even a returning a simple HashMap would be returning a wrapper. as
long as we make the subkey business configurable (with it off when in
deprecation support mode), and only return the
ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
that start with "foo.", then i think we'll be fine.
<snip/>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in
/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools:
generic/ValueParser.java view/ParameterTool.java
Posted by Claude Brisson <cl...@renegat.net>.
Le lundi 12 novembre 2007 à 10:53 -0800, Nathan Bubna a écrit :
> it's not the name so much as how it's used. it looks to me like once
> it searches for subkey "foo", it won't even try to find subkey "bar"
> or "woogie" or whatever. either that's wrong or i'm missing
> something?
The "hasSubkeys" boolean is here so that we don't search for subkeys
twice if we already know there isn't any (so that the overhead when
looking for inexisting keys, in templates that don't use subkeys, is
reduced to only one indeOf('.') in all key names). The very first
implementation was buggy, it may be the one you read.
> > > i'm also curious about what the advantages are of this approach (as
> > > opposed to a ValueParserSub). it's been a while since i've thought
> > > through the implementation options for this.
> >
> > Things have changed since then: the engine is now quite efficient in
> > automatic types conversions so the "foo.int" syntax is much less
> > necessary than "foo.bar", and you always have "foo.getInt()".
>
> huh? foo.int is the same as foo.getInt(). you can't have the latter
> if the former doesn't work. did you mean getInt('foo')?
Yes, sorry.
> also, the
> sub could handle both foo.bar and foo.int, and the engine can only do
> a small fraction of the conversions the ValueParser is capable of.
> so, while i don't see anything wrong with the current approach (apart
> from the hasSubKey thing), i still suspect a Sub has more potential,
> since we have complete control of the API.
>
> > But above all, having the generic getter return something else (that is,
> > the ValueParserSub) than the underlying basic type leads - at least in
> > my experience - to several side effects (like a jdbc driver being unable
> > to handle the ValueParserSub class by calling toString on it).
>
> not sure how returning ValueParserSub is that different from returning
> ValueParser.
We only return a ValueParser for get("foo") when there are "foo.bar"
keys, and keep returning the integral type otherwise. Since we expect
the same fonctionnalities for $params and $params.foo, the recursive
approach looked simpler to me.
Btw, I hoped that by having ValueParser implement Map I'd see
ValueParser objects displayed like "{ key=value ... }" but I still see
the ugly org.apache.velocity.tools.generic.ValueParser@848ecc, I'll dig
into this...
> > The proposed implementation only returns a new ValueParser object
> > whenever subkeys are allowed and found.
>
> same could be done with a Sub. granted, the implementation would have
> to be smarter (i.e. search for a potential matching subkeys first)
> than the last implementation for this, but there's no reason it can't
> be done.
Sure, but it all boils down to the decision to keep the foo.int syntax
or not. If yes, then we definitely need a ValueParserSub object, if no,
then returning either a new ValueParser (when subkeys are found) or the
value in its integral type looks more natural to me.
My feeling here is that although foo.int is a cool syntax, it has too
many backwards ; especially, we should always return the integral type
(string, boolean or number) when available rather than a wrapper around
it to avoid nasty side effects.
Claude
> >
> >
> > Claude
> >
> >
> > > On Nov 9, 2007 7:15 AM, Claude Brisson <cl...@renegat.net> wrote:
> > > > ValueParser now has protected get/setAllowSubkeys() boolean methods.
> > > >
> > > > The default value of allowSubkey shoud be the value of deprecatedMode,
> > > > but I'm not really sure of how this should be done. But I'm sure Nathan
> > > > will be of some help here.
> > > >
> > > > Also, since ValueParser is used internally by the Tool.configure method,
> > > > it may be safer to move all this new stuff in ParameterParser only.
> > > >
> > > > Claude
> > > >
> > > >
> > > > Le vendredi 09 novembre 2007 à 14:49 +0000, cbrisson@apache.org a
> > > > écrit :
> > > >
> > > > > Author: cbrisson
> > > > > Date: Fri Nov 9 06:49:36 2007
> > > > > New Revision: 593549
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=593549&view=rev
> > > > > Log:
> > > > > sub keys getter (not yet tested but doesnt break anything at least)
> > > > >
> > > > > Modified:
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > >
> > > > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > > ==============================================================================
> > > > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java (original)
> > > > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java Fri Nov 9 06:49:36 2007
> > > > > @@ -21,6 +21,9 @@
> > > > >
> > > > > import java.util.Map;
> > > > > import java.util.Locale;
> > > > > +import java.util.Set;
> > > > > +import java.util.HashMap;
> > > > > +
> > > > > import org.apache.velocity.tools.config.DefaultKey;
> > > > >
> > > > > /**
> > > > > @@ -40,6 +43,14 @@
> > > > > {
> > > > > private Map source = null;
> > > > >
> > > > > + private boolean allowSubkeys = true; /* default to whatever, should be overridden by depreprecatedMode default value anyway */
> > > > > +
> > > > > + /* when using subkeys, cache at least the presence of any subkey,
> > > > > + so that the rendering of templates not using subkeys will only
> > > > > + look once for subkeys
> > > > > + */
> > > > > + private boolean hasSubkeys = true;
> > > > > +
> > > > > public ValueParser() {}
> > > > >
> > > > > public ValueParser(Map source)
> > > > > @@ -98,7 +109,11 @@
> > > > > {
> > > > > return null;
> > > > > }
> > > > > - return getSource().get(key);
> > > > > + Object value = getSource().get(key);
> > > > > + if (value == null && getAllowSubkeys()) {
> > > > > + value = getSubkey(key);
> > > > > + }
> > > > > + return value;
> > > > > }
> > > > >
> > > > > public Object[] getValues(String key)
> > > > > @@ -359,4 +374,29 @@
> > > > > return toLocales(getValues(key));
> > > > > }
> > > > >
> > > > > + protected boolean getAllowSubkeys() {
> > > > > + return allowSubkeys;
> > > > > + }
> > > > > +
> > > > > + protected void setAllowSubkeys(boolean allow) {
> > > > > + allowSubkeys = allow;
> > > > > + }
> > > > > +
> > > > > + protected Object getSubkey(String subkey) {
> > > > > + if (!hasSubkeys || subkey == null || subkey.length() == 0) {
> > > > > + return null;
> > > > > + }
> > > > > + Map<String,Object> values = null;
> > > > > + subkey = subkey.concat(".");
> > > > > + for(Map.Entry<String,Object> entry:(Set<Map.Entry>)getSource().entrySet()) {
> > > > > + if(entry.getKey().startsWith(subkey)) {
> > > > > + if(values == null) {
> > > > > + values = new HashMap<String,Object>();
> > > > > + }
> > > > > + values.put(entry.getKey().substring(subkey.length()),entry.getValue());
> > > > > + }
> > > > > + }
> > > > > + hasSubkeys = (values == null);
> > > > > + return values;
> > > > > + }
> > > > > }
> > > > >
> > > > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > > ==============================================================================
> > > > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java (original)
> > > > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java Fri Nov 9 06:49:36 2007
> > > > > @@ -109,7 +109,11 @@
> > > > > */
> > > > > public Object getValue(String key)
> > > > > {
> > > > > - return getRequest().getParameter(key);
> > > > > + Object value = getRequest().getParameter(key);
> > > > > + if(value == null && getAllowSubkeys()) {
> > > > > + value = getSubkey(key);
> > > > > + }
> > > > > + return value;
> > > > > }
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > > For additional commands, e-mail: dev-help@velocity.apache.org
> > > >
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > For additional commands, e-mail: dev-help@velocity.apache.org
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > For additional commands, e-mail: dev-help@velocity.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: generic/ValueParser.java view/ParameterTool.java
Posted by Nathan Bubna <nb...@gmail.com>.
On Nov 10, 2007 2:54 AM, Claude Brisson <cl...@renegat.net> wrote:
> Le vendredi 09 novembre 2007 à 12:15 -0800, Nathan Bubna a écrit :
> > - the hasSubKeys things doesn't make any sense to me
>
> Maybe the name isn't appropriate. What I call "subkey" is "foo" or "bar"
> in "foo.bar". Feel free to rename anything, English is not my mother
> tongue...
it's not the name so much as how it's used. it looks to me like once
it searches for subkey "foo", it won't even try to find subkey "bar"
or "woogie" or whatever. either that's wrong or i'm missing
something?
> > i'm also curious about what the advantages are of this approach (as
> > opposed to a ValueParserSub). it's been a while since i've thought
> > through the implementation options for this.
>
> Things have changed since then: the engine is now quite efficient in
> automatic types conversions so the "foo.int" syntax is much less
> necessary than "foo.bar", and you always have "foo.getInt()".
huh? foo.int is the same as foo.getInt(). you can't have the latter
if the former doesn't work. did you mean getInt('foo')? also, the
sub could handle both foo.bar and foo.int, and the engine can only do
a small fraction of the conversions the ValueParser is capable of.
so, while i don't see anything wrong with the current approach (apart
from the hasSubKey thing), i still suspect a Sub has more potential,
since we have complete control of the API.
> But above all, having the generic getter return something else (that is,
> the ValueParserSub) than the underlying basic type leads - at least in
> my experience - to several side effects (like a jdbc driver being unable
> to handle the ValueParserSub class by calling toString on it).
not sure how returning ValueParserSub is that different from returning
ValueParser.
> The proposed implementation only returns a new ValueParser object
> whenever subkeys are allowed and found.
same could be done with a Sub. granted, the implementation would have
to be smarter (i.e. search for a potential matching subkeys first)
than the last implementation for this, but there's no reason it can't
be done.
>
>
> Claude
>
>
> > On Nov 9, 2007 7:15 AM, Claude Brisson <cl...@renegat.net> wrote:
> > > ValueParser now has protected get/setAllowSubkeys() boolean methods.
> > >
> > > The default value of allowSubkey shoud be the value of deprecatedMode,
> > > but I'm not really sure of how this should be done. But I'm sure Nathan
> > > will be of some help here.
> > >
> > > Also, since ValueParser is used internally by the Tool.configure method,
> > > it may be safer to move all this new stuff in ParameterParser only.
> > >
> > > Claude
> > >
> > >
> > > Le vendredi 09 novembre 2007 à 14:49 +0000, cbrisson@apache.org a
> > > écrit :
> > >
> > > > Author: cbrisson
> > > > Date: Fri Nov 9 06:49:36 2007
> > > > New Revision: 593549
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=593549&view=rev
> > > > Log:
> > > > sub keys getter (not yet tested but doesnt break anything at least)
> > > >
> > > > Modified:
> > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > >
> > > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > ==============================================================================
> > > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java (original)
> > > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java Fri Nov 9 06:49:36 2007
> > > > @@ -21,6 +21,9 @@
> > > >
> > > > import java.util.Map;
> > > > import java.util.Locale;
> > > > +import java.util.Set;
> > > > +import java.util.HashMap;
> > > > +
> > > > import org.apache.velocity.tools.config.DefaultKey;
> > > >
> > > > /**
> > > > @@ -40,6 +43,14 @@
> > > > {
> > > > private Map source = null;
> > > >
> > > > + private boolean allowSubkeys = true; /* default to whatever, should be overridden by depreprecatedMode default value anyway */
> > > > +
> > > > + /* when using subkeys, cache at least the presence of any subkey,
> > > > + so that the rendering of templates not using subkeys will only
> > > > + look once for subkeys
> > > > + */
> > > > + private boolean hasSubkeys = true;
> > > > +
> > > > public ValueParser() {}
> > > >
> > > > public ValueParser(Map source)
> > > > @@ -98,7 +109,11 @@
> > > > {
> > > > return null;
> > > > }
> > > > - return getSource().get(key);
> > > > + Object value = getSource().get(key);
> > > > + if (value == null && getAllowSubkeys()) {
> > > > + value = getSubkey(key);
> > > > + }
> > > > + return value;
> > > > }
> > > >
> > > > public Object[] getValues(String key)
> > > > @@ -359,4 +374,29 @@
> > > > return toLocales(getValues(key));
> > > > }
> > > >
> > > > + protected boolean getAllowSubkeys() {
> > > > + return allowSubkeys;
> > > > + }
> > > > +
> > > > + protected void setAllowSubkeys(boolean allow) {
> > > > + allowSubkeys = allow;
> > > > + }
> > > > +
> > > > + protected Object getSubkey(String subkey) {
> > > > + if (!hasSubkeys || subkey == null || subkey.length() == 0) {
> > > > + return null;
> > > > + }
> > > > + Map<String,Object> values = null;
> > > > + subkey = subkey.concat(".");
> > > > + for(Map.Entry<String,Object> entry:(Set<Map.Entry>)getSource().entrySet()) {
> > > > + if(entry.getKey().startsWith(subkey)) {
> > > > + if(values == null) {
> > > > + values = new HashMap<String,Object>();
> > > > + }
> > > > + values.put(entry.getKey().substring(subkey.length()),entry.getValue());
> > > > + }
> > > > + }
> > > > + hasSubkeys = (values == null);
> > > > + return values;
> > > > + }
> > > > }
> > > >
> > > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > ==============================================================================
> > > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java (original)
> > > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java Fri Nov 9 06:49:36 2007
> > > > @@ -109,7 +109,11 @@
> > > > */
> > > > public Object getValue(String key)
> > > > {
> > > > - return getRequest().getParameter(key);
> > > > + Object value = getRequest().getParameter(key);
> > > > + if(value == null && getAllowSubkeys()) {
> > > > + value = getSubkey(key);
> > > > + }
> > > > + return value;
> > > > }
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > > For additional commands, e-mail: dev-help@velocity.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > For additional commands, e-mail: dev-help@velocity.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r593549 - in
/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools:
generic/ValueParser.java view/ParameterTool.java
Posted by Claude Brisson <cl...@renegat.net>.
Le vendredi 09 novembre 2007 à 12:15 -0800, Nathan Bubna a écrit :
> - the hasSubKeys things doesn't make any sense to me
Maybe the name isn't appropriate. What I call "subkey" is "foo" or "bar"
in "foo.bar". Feel free to rename anything, English is not my mother
tongue...
> i'm also curious about what the advantages are of this approach (as
> opposed to a ValueParserSub). it's been a while since i've thought
> through the implementation options for this.
Things have changed since then: the engine is now quite efficient in
automatic types conversions so the "foo.int" syntax is much less
necessary than "foo.bar", and you always have "foo.getInt()".
But above all, having the generic getter return something else (that is,
the ValueParserSub) than the underlying basic type leads - at least in
my experience - to several side effects (like a jdbc driver being unable
to handle the ValueParserSub class by calling toString on it).
The proposed implementation only returns a new ValueParser object
whenever subkeys are allowed and found.
Claude
> On Nov 9, 2007 7:15 AM, Claude Brisson <cl...@renegat.net> wrote:
> > ValueParser now has protected get/setAllowSubkeys() boolean methods.
> >
> > The default value of allowSubkey shoud be the value of deprecatedMode,
> > but I'm not really sure of how this should be done. But I'm sure Nathan
> > will be of some help here.
> >
> > Also, since ValueParser is used internally by the Tool.configure method,
> > it may be safer to move all this new stuff in ParameterParser only.
> >
> > Claude
> >
> >
> > Le vendredi 09 novembre 2007 à 14:49 +0000, cbrisson@apache.org a
> > écrit :
> >
> > > Author: cbrisson
> > > Date: Fri Nov 9 06:49:36 2007
> > > New Revision: 593549
> > >
> > > URL: http://svn.apache.org/viewvc?rev=593549&view=rev
> > > Log:
> > > sub keys getter (not yet tested but doesnt break anything at least)
> > >
> > > Modified:
> > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > >
> > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java?rev=593549&r1=593548&r2=593549&view=diff
> > > ==============================================================================
> > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java (original)
> > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java Fri Nov 9 06:49:36 2007
> > > @@ -21,6 +21,9 @@
> > >
> > > import java.util.Map;
> > > import java.util.Locale;
> > > +import java.util.Set;
> > > +import java.util.HashMap;
> > > +
> > > import org.apache.velocity.tools.config.DefaultKey;
> > >
> > > /**
> > > @@ -40,6 +43,14 @@
> > > {
> > > private Map source = null;
> > >
> > > + private boolean allowSubkeys = true; /* default to whatever, should be overridden by depreprecatedMode default value anyway */
> > > +
> > > + /* when using subkeys, cache at least the presence of any subkey,
> > > + so that the rendering of templates not using subkeys will only
> > > + look once for subkeys
> > > + */
> > > + private boolean hasSubkeys = true;
> > > +
> > > public ValueParser() {}
> > >
> > > public ValueParser(Map source)
> > > @@ -98,7 +109,11 @@
> > > {
> > > return null;
> > > }
> > > - return getSource().get(key);
> > > + Object value = getSource().get(key);
> > > + if (value == null && getAllowSubkeys()) {
> > > + value = getSubkey(key);
> > > + }
> > > + return value;
> > > }
> > >
> > > public Object[] getValues(String key)
> > > @@ -359,4 +374,29 @@
> > > return toLocales(getValues(key));
> > > }
> > >
> > > + protected boolean getAllowSubkeys() {
> > > + return allowSubkeys;
> > > + }
> > > +
> > > + protected void setAllowSubkeys(boolean allow) {
> > > + allowSubkeys = allow;
> > > + }
> > > +
> > > + protected Object getSubkey(String subkey) {
> > > + if (!hasSubkeys || subkey == null || subkey.length() == 0) {
> > > + return null;
> > > + }
> > > + Map<String,Object> values = null;
> > > + subkey = subkey.concat(".");
> > > + for(Map.Entry<String,Object> entry:(Set<Map.Entry>)getSource().entrySet()) {
> > > + if(entry.getKey().startsWith(subkey)) {
> > > + if(values == null) {
> > > + values = new HashMap<String,Object>();
> > > + }
> > > + values.put(entry.getKey().substring(subkey.length()),entry.getValue());
> > > + }
> > > + }
> > > + hasSubkeys = (values == null);
> > > + return values;
> > > + }
> > > }
> > >
> > > Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java?rev=593549&r1=593548&r2=593549&view=diff
> > > ==============================================================================
> > > --- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java (original)
> > > +++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java Fri Nov 9 06:49:36 2007
> > > @@ -109,7 +109,11 @@
> > > */
> > > public Object getValue(String key)
> > > {
> > > - return getRequest().getParameter(key);
> > > + Object value = getRequest().getParameter(key);
> > > + if(value == null && getAllowSubkeys()) {
> > > + value = getSubkey(key);
> > > + }
> > > + return value;
> > > }
> > >
> > >
> > >
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> > For additional commands, e-mail: dev-help@velocity.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org