ok, so here's my current thought. since we've already changed this in
the last production release, i'm hesitant to just "change our mind"
and leave the VELOCITY-285 advocates hanging. so, here's two options
i think i'd be comfortable with:
1:
Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
===================================================================
--- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(revision 701169)
+++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(working copy)
@@ -247,6 +247,15 @@
}
/**
+ * Extension hook to allow VELOCITY-285 behavior by overriding
+ * and using context.localPut(key, value). See VELOCITY-630.
+ */
+ protected void put(InternalContextAdapter context, String key,
Object value)
+ {
+ context.put(key, value);
+ }
+
+ /**
* renders the #foreach() block
* @param context
* @param writer
@@ -325,11 +334,11 @@
while (!maxNbrLoopsExceeded && i.hasNext())
{
- // TODO: JDK 1.4+ -> Integer.valueOf()
- context.localPut(counterName , new Integer(counter));
- context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
+ // TODO: JDK 1.5+ -> Integer.valueOf()
+ put(context, counterName , new Integer(counter));
+ put(context, hasNextName, Boolean.valueOf(i.hasNext()));
Object value = i.next();
- context.localPut(elementKey, value);
+ put(context, elementKey, value);
or 2:
Index: ../src/java/org/apache/velocity/runtime/directive/Foreach.java
===================================================================
--- ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(revision 701169)
+++ ../src/java/org/apache/velocity/runtime/directive/Foreach.java
(working copy)
@@ -177,6 +177,11 @@
private boolean skipInvalidIterator;
/**
+ * Whether or not [EMAIL PROTECTED] #put} uses context.put or
context.localPut
+ */
+ private boolean strictPassByName;
+
+ /**
* The reference name used to access each
* of the elements in the list object. It
* is the $item in the following:
@@ -217,6 +222,7 @@
}
skipInvalidIterator =
rsvc.getBoolean(RuntimeConstants.SKIP_INVALID_ITERATOR, true);
+ strictPassByName =
rsvc.getBoolean(RuntimeConstants.STRICT_FOREACH_PASS_BY_NAME, true);
/*
* this is really the only thing we can do here as everything
@@ -247,6 +253,22 @@
}
/**
+ * Controls whether loop vars are set locally or generally. See
VELOCITY-630
+ * and VELOCITY-285.
+ */
+ protected void put(InternalContextAdapter context, String key,
Object value)
+ {
+ if (strictPassByName)
+ {
+ context.put(key, value);
+ }
+ else
+ {
+ context.localPut(key, value);
+ }
+ }
+
+ /**
* renders the #foreach() block
* @param context
* @param writer
@@ -325,11 +347,11 @@
while (!maxNbrLoopsExceeded && i.hasNext())
{
- // TODO: JDK 1.4+ -> Integer.valueOf()
- context.localPut(counterName , new Integer(counter));
- context.localPut(hasNextName, Boolean.valueOf(i.hasNext()));
+ // TODO: JDK 1.5+ -> Integer.valueOf()
+ put(context, counterName , new Integer(counter));
+ put(context, hasNextName, Boolean.valueOf(i.hasNext()));
Object value = i.next();
- context.localPut(elementKey, value);
+ put(context, elementKey, value);
On Tue, Oct 14, 2008 at 9:40 PM, Nathan Bubna <[EMAIL PROTECTED]> wrote:
> hmm. i suppose that makes sense... of course, i'm too tired to think
> straight right now. let me re-examine this tomorrow.
>
> On Tue, Oct 14, 2008 at 6:39 PM, Byron Foster <[EMAIL PROTECTED]> wrote:
>> So I looked into this some more and here is the deal. This bug was
>> introduced with VELOCITY-285. The solution to 285 was to make the #foreach
>> index variable forced to local scope, which introduced bug VELOCITY-630.
>> However, I assert that 285 was never a bug!
>>
>> Given the following VTL:
>>
>> #macro(dig $x)
>> x1: $x
>> #foreach($i in $x)
>> x2 $x
>> #dig($i)
>> #end
>> #end
>> #dig([[1,2],[3,4]])
>>
>> The premiss of 285 is that reference $x at x1: and x2: should render with
>> the same value. However, in a pass by name world this is not true. On at
>> least the second recursion level to #dig the reference $x is essentially an
>> alias for $i. Since $i changes value between x1 and x2, rendering $x will
>> change.
>>
>> I agree with the sentiment that this is surprising, but it is working as it
>> should.
>>
>> On Oct 14, 2008, at 1:58 , Byron Foster wrote:
>>
>>> Actually I don't think it's all that odd, the error case is somewhat
>>> general (it came up in my own usage of Velocity to generate HTML lists). Any
>>> pass by name reference to the #foreach iteration variable will fail, it
>>> doesn't need to be in a string literal, for example:
>>>
>>> #macro(test $x)#foreach($i in [1, 2])$x#end#end
>>> #test($i)
>>>
>>> Maybe a different disscussion, but somewhat related to how #foreach is
>>> implemented with local scope. The macro localscope setting
>>> (velocimacro.context.localscope) has the same behavior as above for the #set
>>> directive. However, I'm not sure what Velocity's intended behavior is with
>>> scoping rules and pass by name.
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [EMAIL PROTECTED]
>> For additional commands, e-mail: [EMAIL PROTECTED]
>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
