Re: Expanding Environmental variables patch

Charlei,

csanders wrote:
> Attached is the patch, adding three new functions,
> expandEnvironmentalVariables( ExtendedProperties ),
> expandEnvironmentalVariables(Vector ),

Any reason not to accept any Collection rather than limiting this to
just a Vector? Forgive me if there are API implications coming from
ExtendedProperties.

> + Vector newVector = new Vector();

This should really be:

Vectory newVector = new Vector(stringVector.size());
(or an appropriate Collection instance)

Better yet, if you are using Vectors, why not use Vector.set() instead
of creating a completely new Vector every time? I actually dislike
modifying passed-in parameters, but your
expandEnvironmentalVariables(ExtendedProperties) method does that
already, so what does it matter if the methods that it calls do the same?

> The expansion should really take place in ExtendedProperties, but this
> might work in the interim .

Then why not patch ExtendedProperties instead?

I'm a little curious why this capability needs to be added to Velocity
in the first place. Does anybody really use environment variables
anymore, anyway?

If anything, this should be implemented as a subclass of
ExtendedProperties and handled there. There is no need to modify any
Velocity code to get this to work.

-chris