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/