Velocity
  1. Velocity
  2. VELOCITY-619

New set modifier so that set only assigns a value when variable is not defined.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: 1.6
    • Component/s: Engine
    • Labels:
      None

      Description

      Define a new #set modifier such that a variable is only set if it is not defined, like so:

      #set($!foo = "bar")

      If the '!' character proceeds the variable foo, then $foo will be set with the value of "bar" ONLY IF $foo is not defined (is not in the context). Otherwise if $foo is defined it will remain unchanged.

        Activity

        Hide
        Byron Foster added a comment -

        Updated version of the patch so that the set modifier plays nice within macros

        Show
        Byron Foster added a comment - Updated version of the patch so that the set modifier plays nice within macros
        Hide
        Byron Foster added a comment -

        The changes to the ProxyVMContext.java file in this patch will conflict with the same changes in the StrictPropertyAndVariable.patch in VELOCITY-619 but this is only because they contains the exact same change, and can be ignored.

        Show
        Byron Foster added a comment - The changes to the ProxyVMContext.java file in this patch will conflict with the same changes in the StrictPropertyAndVariable.patch in VELOCITY-619 but this is only because they contains the exact same change, and can be ignored.
        Hide
        Nathan Bubna added a comment -

        I have mixed feelings about this.

        First, there are some backwards compatibility concerns. Since #set( $!foo = 2 ) always worked (though the bang was meaningless), this could break things for people who unwittingly use(d) that syntax already. That's not a showstopper, as they had no good reason to do so, but we would have to make sure to highlight such a change in the changelog, log a debug message each time, and perhaps even make this configurable.

        Second, it feels odd to me that #set( $!foo = 2 ) should work, but not #set( $!foo.bar = 2 ). Your comments in the patch simply say "it makes no sense", but why not? It seems like it's more a matter of being tricky to implement the check for the value. I think if we were to do this, it would be best to be as consistent about it as possible.

        Anyone else have thoughts about this?

        Show
        Nathan Bubna added a comment - I have mixed feelings about this. First, there are some backwards compatibility concerns. Since #set( $!foo = 2 ) always worked (though the bang was meaningless), this could break things for people who unwittingly use(d) that syntax already. That's not a showstopper, as they had no good reason to do so, but we would have to make sure to highlight such a change in the changelog, log a debug message each time, and perhaps even make this configurable. Second, it feels odd to me that #set( $!foo = 2 ) should work, but not #set( $!foo.bar = 2 ). Your comments in the patch simply say "it makes no sense", but why not? It seems like it's more a matter of being tricky to implement the check for the value. I think if we were to do this, it would be best to be as consistent about it as possible. Anyone else have thoughts about this?
        Hide
        Christoph Reck added a comment -

        True that if someone used #set( $!foo = $bar ) in an earlier version it set the value ignoring the bang. At some point it was discussed to use this syntax to explicetely allow setting a NULL value. Changing this would break BC (even though it is an unimplemented feature - some people may have local patches to do so - as I had once...).

        I propose to add a new directive for this, instead of overloading the #set with bang to mean "set only if undefined".
        Possible directive names: #default, #option, #setUndefined, #setIfUndefined, #allow, #let

        P.S. This is inline with some new directives introduced in Vel1.6: #evaluate, #define($ref)...#end, and #break

        Show
        Christoph Reck added a comment - True that if someone used #set( $!foo = $bar ) in an earlier version it set the value ignoring the bang. At some point it was discussed to use this syntax to explicetely allow setting a NULL value. Changing this would break BC (even though it is an unimplemented feature - some people may have local patches to do so - as I had once...). I propose to add a new directive for this, instead of overloading the #set with bang to mean "set only if undefined". Possible directive names: #default, #option, #setUndefined, #setIfUndefined, #allow, #let P.S. This is inline with some new directives introduced in Vel1.6: #evaluate, #define($ref)...#end, and #break
        Hide
        Byron Foster added a comment -

        Nathan, The reason I comment that "It makes no sense" is that in my interpretation the bang modifier instructs the set directive to only assign a value if the variable is undefined. So In the situation:

        #set($!foo.bar = 2)

        If we are NOT in strict mode, and 'bar' does not exist, then this statement will be ignored.
        If we are in strict mode, and 'bar' does not exist, then an exception will be thrown.

        Ultimately, In the above case, the bang modifier will never change the behavior of the set directive, but maybe throwing an exception is to draconian, and just letting it go is the better option.

        Show
        Byron Foster added a comment - Nathan, The reason I comment that "It makes no sense" is that in my interpretation the bang modifier instructs the set directive to only assign a value if the variable is undefined. So In the situation: #set($!foo.bar = 2) If we are NOT in strict mode, and 'bar' does not exist, then this statement will be ignored. If we are in strict mode, and 'bar' does not exist, then an exception will be thrown. Ultimately, In the above case, the bang modifier will never change the behavior of the set directive, but maybe throwing an exception is to draconian, and just letting it go is the better option.
        Hide
        Byron Foster added a comment -

        My main motivation for this patch is to provide a need for this functionality when running in strict mode VELOCITY-618 Mainly it would be good to allow the developer to provide default values for variables that are referenced but not defined in the template, without the need to add them to the context in java code.

        An alternative to this method would be to provide an exception in strict mode in the case of #if($foo). In the case of just a single variable argument in #if strict mode would not throw an exception in the case $foo is not defined, and the #if would fail. so the following could be done (As it can be now).

        #if ( ! $foo) #set($foo = "bar") #end

        My only concern with this is that it is an exception, which means to me a "special case" which often times leads to confusion in behavior. But perhaps it's justified in this situation.

        Show
        Byron Foster added a comment - My main motivation for this patch is to provide a need for this functionality when running in strict mode VELOCITY-618 Mainly it would be good to allow the developer to provide default values for variables that are referenced but not defined in the template, without the need to add them to the context in java code. An alternative to this method would be to provide an exception in strict mode in the case of #if($foo). In the case of just a single variable argument in #if strict mode would not throw an exception in the case $foo is not defined, and the #if would fail. so the following could be done (As it can be now). #if ( ! $foo) #set($foo = "bar") #end My only concern with this is that it is an exception, which means to me a "special case" which often times leads to confusion in behavior. But perhaps it's justified in this situation.
        Hide
        Nathan Bubna added a comment -

        Not being able to use #if like that in strict mode might confuse people too. I actually didn't fully realize/notice that the patch for VELOCITY-618 would make #if( $foo ) throw an error in strict mode. i use #if to check if a variable is present as often as i use it to check if a variable is true or false.

        My preference would be to allow #if( $foo ) even in strict mode. Then this behaviour is possible without introducing any backwards compatibility concerns. We can re-examine this if/when someone gets a 2.0 version started. Then backwards compatibility is not much of a concern.

        Show
        Nathan Bubna added a comment - Not being able to use #if like that in strict mode might confuse people too. I actually didn't fully realize/notice that the patch for VELOCITY-618 would make #if( $foo ) throw an error in strict mode. i use #if to check if a variable is present as often as i use it to check if a variable is true or false. My preference would be to allow #if( $foo ) even in strict mode. Then this behaviour is possible without introducing any backwards compatibility concerns. We can re-examine this if/when someone gets a 2.0 version started. Then backwards compatibility is not much of a concern.
        Hide
        Byron Foster added a comment -

        Ok, I'm convinced this is not the right way to do this.

        Show
        Byron Foster added a comment - Ok, I'm convinced this is not the right way to do this.

          People

          • Assignee:
            Unassigned
            Reporter:
            Byron Foster
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development