On 01/14/2011 04:04 PM, Sergiu Dumitriu wrote:
On 01/14/2011 09:06 AM, Vincent Massol wrote:
Hi Sergiu,
I'd like to discuss this point that you've made in some other mail:
"Velocity 1.7, which brings many new features and improvements, but
breaks a lot of velocity macros (still working on fixing the broken
code)"
Does this mean users who have existing scripts will have things not
working anymore? (I guess so if we have broken stuff on our side)
If so, what can we do to make them suffer less when upgrading? Can we
start by listing broken stuff here and add them to the release
notes?
The key point is that Velocity 1.7 changed the way macro evaluations
work. While before it was more flexible with many possible outcomes
depending on what parameters were passed, and how they were used inside
the macro, the current version simplified a lot the internal logic of
variable assignments inside macros, which resulted in a critical
regression for us. The same change was introduced betwee 1.6.0 and
1.6.1, but was reverted in 1.6.2 when we notified them of the
regression, with the decision to go further with the change in 1.7.
To better understand the kind of code that doesn't work, take this example:
#macro(callBySharing $x)
#set($x = 'a')
#end
#set($y = 'y')
#callBySharing($y)
$y -> 'y' in 1.7
$y -> 'a' in 1.6.2, 1.6.0 and before)
But:
#set($x = 'x')
#callBySharing($x)
$x -> 'a' in all versions
This means that only macros that are supposed to change and return a
value in one of its formal parameters will stop working, and only when
the formal and actual parameters have different names. Macros with
signatures like:
#macro(computeSomething $fromValue1 $fromValue2 $putResultHere)
The only macro in macros.vm that was broken by this change was
#setVariableFromRequest, which I already fixed.
I also added a generic #setVariable ("variableName" $value) macro which
can emulate the call by sharing behavior. How to use it:
Suppose you had a macro like this:
#macro(isBlogGlobal $blogDoc $isGlobal)
#set($isGlobal = false)
#getBlogProperty($blogDoc 'blogType' '' $discard)
#if($discard == 'global')
#set($isGlobal = true)
#end
#end
Here $isGlobal is the output variable which now doesn't always work. The
updated version of the macro can be written as:
#macro(isBlogGlobal $blogDoc $isGlobal)
#set ($result = false)
#getBlogProperty($blogDoc 'blogType' '' $discard)
#if($discard == 'global')
#set($result = true)
#end
#set ($isGlobal = $util.null)
#setVariable ("$isGlobal" $result)
#end
Pay attention to the last two lines in the macro.
In Velocity, when rendering $variable, where $variable is undefined or
null, will cause the variable name to be printed instead. As it happens,
when inside a macro, what gets printed is the name of the actual
parameter (the one passed in the macro call), and not the formal one
(the one declared in the macro definition). So, whenever $isGlobal is
rendered as a string, the name of the actual parameter is obtained.
#set ($isGlobal = $util.null) will make sure that no matter what the
previous value of the variable was, $isGlobal will be null from this
point forward, and "$isGlobal" will print the name of the actual parameter.
When calling #setVariable ("$isGlobal" $result), the first parameter
will contain the name of the actual parameter used when calling
#isBlogGlobal.
Inside the #setVariable macro, the wanted variable is assigned using
#evaluate.
There are a lot of broken macros in the blog
application, where I made
extensive use of this kind of macro behavior, which I'm going to fix. I
don't think that there are many other places in XE that were broken, and
I doubt that much user code makes use of this behavior.
Fixed.
Still, if we want to prevent any breakages, keep in mind that Velocity
will not change back to the old behavior. We can either stick to 1.6
till the end of time, keep patching 1.7 and any future release to keep
the behavior in place, or accept the change and document the breakage.
A simple solution for the moment is to patch Velocity 1.7 during the 3.x
cycle, but I'd rather not do it. 3.0 is the start of a new cycle, and
it's the right time to make major changes.
--
Sergiu Dumitriu
http://purl.org/net/sergiu/