Velocity
  1. Velocity
  2. VELOCITY-623

Modify escape behavior in strict mode

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.7
    • Component/s: Engine
    • Labels:
      None

      Description

      provides another property 'runtime.references.strict.escape' that when true modifies the behavior of escaping a reference when in strict mode, see discussion VELOCITY-618. The behavior is a as follows:

      $abc = <exception> - If $abc is not defined
      \$abc = $abc
      $abc = <exception> - If $abc is not defined
      \\\$abc = \$abc

      The backslash in front of a reference always prevents that reference from being evaluated, and the backslash is removed. This is true wether a reference is in the context or not.

        Issue Links

          Activity

          Hide
          Nathan Bubna added a comment -

          Is there any particular reason the escaping has to be related to strict mode? This is an improvement, as it shows the user intended to change escaping behavior. But, could this change in escaping behavior work even without strict mode on? Also, what about escaping directives and macros?

          Show
          Nathan Bubna added a comment - Is there any particular reason the escaping has to be related to strict mode? This is an improvement, as it shows the user intended to change escaping behavior. But, could this change in escaping behavior work even without strict mode on? Also, what about escaping directives and macros?
          Hide
          Byron Foster added a comment -

          My thoughts are that no, there is no particular reason. Macro's are a problem because the parser handles the escaping such that no node is even created for something like "#abc() which makes it impossible to do the post parse processing that can be done with references. If it wasn't for this I would have also "fixed" the macro escaping for strict mode To change the behavior of macro escaping the parser will have to be changed, However, not a major change...

          In allot of ways strict mode along with sane escaping rules really simplifies things because then you either have a valid reference with no escape processing necessary, or a text node (maybe actually, or essentially). As it stands the ASTReference Node has allot of member fields dedicated to various escape gymnastics. I'm not even sure what all the bang escaping is for.

          I think were you are going with this is that it would be nice to have a property that modifies the behavior of escaping, and perhaps tests how it will work in the future, in a python future sort of way. If this is the case it would be good to get it all right. Anyway, I'll look into it some more.

          Show
          Byron Foster added a comment - My thoughts are that no, there is no particular reason. Macro's are a problem because the parser handles the escaping such that no node is even created for something like "#abc() which makes it impossible to do the post parse processing that can be done with references. If it wasn't for this I would have also "fixed" the macro escaping for strict mode To change the behavior of macro escaping the parser will have to be changed, However, not a major change... In allot of ways strict mode along with sane escaping rules really simplifies things because then you either have a valid reference with no escape processing necessary, or a text node (maybe actually, or essentially). As it stands the ASTReference Node has allot of member fields dedicated to various escape gymnastics. I'm not even sure what all the bang escaping is for. I think were you are going with this is that it would be nice to have a property that modifies the behavior of escaping, and perhaps tests how it will work in the future, in a python future sort of way. If this is the case it would be good to get it all right. Anyway, I'll look into it some more.
          Hide
          Nathan Bubna added a comment -

          Targeting for 1.7. I'm still thinking that i'd at least want this work the same for directives/macros as for references, before we commit it.

          Show
          Nathan Bubna added a comment - Targeting for 1.7. I'm still thinking that i'd at least want this work the same for directives/macros as for references, before we commit it.
          Hide
          Byron Foster added a comment -

          The parser needs to be changed for this, and most of the work involved I believe would be to maintain backward compatibility. I would rather make the change when backward compatibility is not an issue.

          Show
          Byron Foster added a comment - The parser needs to be changed for this, and most of the work involved I believe would be to maintain backward compatibility. I would rather make the change when backward compatibility is not an issue.
          Hide
          Nathan Bubna added a comment -

          Considering that there seem to be more changes hung up on backward compatibility than not, it might be time to open a 2.x branch after we get 1.6 released, and limit 1.7 or 1.6.x to small stuff.

          Show
          Nathan Bubna added a comment - Considering that there seem to be more changes hung up on backward compatibility than not, it might be time to open a 2.x branch after we get 1.6 released, and limit 1.7 or 1.6.x to small stuff.
          Hide
          Byron Foster added a comment -

          I'm going to close this because it's related to the 2.0 issue of escape syntax, which is better discussed in an issue that deals with the problem in general. And yea, this should work for both macros and references.

          Show
          Byron Foster added a comment - I'm going to close this because it's related to the 2.0 issue of escape syntax, which is better discussed in an issue that deals with the problem in general. And yea, this should work for both macros and references.
          Hide
          Byron Foster added a comment -

          I reopened this because with more familiarity with macro parsing, I can make this work for macros also.

          Do we want to stick with given property name?

          Show
          Byron Foster added a comment - I reopened this because with more familiarity with macro parsing, I can make this work for macros also. Do we want to stick with given property name?
          Hide
          Nathan Bubna added a comment -

          Yeah, the proposed property name is fine. And i'm fine with this so long as it's configurable and works consistently for macros, directives and references. If you can do it now, that'd be great.

          Show
          Nathan Bubna added a comment - Yeah, the proposed property name is fine. And i'm fine with this so long as it's configurable and works consistently for macros, directives and references. If you can do it now, that'd be great.
          Hide
          Byron Foster added a comment -

          I wasn't sure what to do about documentation. I added the standard blurb about the property in the developer docs. I think it will only serve to confuse by adding mention of this in the user's docs along with the already lengthy and confusing escape behavior description.

          Show
          Byron Foster added a comment - I wasn't sure what to do about documentation. I added the standard blurb about the property in the developer docs. I think it will only serve to confuse by adding mention of this in the user's docs along with the already lengthy and confusing escape behavior description.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development