Velocity
  1. Velocity
  2. VELOCITY-565

EvaluateContext does not take account of inner.localContext

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6
    • Component/s: None
    • Labels:
      None

      Description

      the following vtl does not render what is, IMHO, expected :

      #macro(testEval $expr)

      #foreach($value in ["val1", "val2"])
      value is : #evaluate( $expr )
      #end
      #end

      #testEval( "$

      {value}" )

      renders :

      value is : ${value}

      value is : $

      {value}

      The reason is that EvaluateContext ctor assigns 'inner.getBaseContext()'
      to instance variable 'innerContext', and getBaseContext() does not
      return VMContext localContext refs (of the #foreach directive).

      Changing the EvaluateContext ctor so that 'innerContext' is set to
      'inner' allow velocity to render the right result :

      value is : val1
      value is : val2

      But I'm not sure this would be a correct fix, should the VMContext
      return all current refs ? Could you tell me what the normal behavior is
      and what is the way all that stuff sould work ?

      Etienne Massip

      1. EvaluateContext.java
        10 kB
        Etienne Massip
      2. evalvmcontext.patch
        3 kB
        Etienne Massip

        Activity

        Hide
        Will Glass-Husain added a comment -

        Etienne--

        Awesome! A bug report and a patch! Appreciate the contribution.

        I'll commit this ASAP if your patch has three things

        --> a change to the unit test that (ideally) breaks before the patch and is fixed after
        --> all the changes (and unit test) is submitted in 'svn diff' format
        --> you verbally assure me that you ran 'ant test' and everything passed.

        Regarding the diff format-- it's easy. Just checkout the latest trunk code from subversion. Make your changes. Go to the root directory and type "svn diff" and capture the output. That should capture in one file the code change and the unit test change.

        If you don't have time to do those things, we still appreciate your effort, but it may be a while until I have time to work through the issue and verify the patch works but doesn't break anything else.

        Show
        Will Glass-Husain added a comment - Etienne-- Awesome! A bug report and a patch! Appreciate the contribution. I'll commit this ASAP if your patch has three things --> a change to the unit test that (ideally) breaks before the patch and is fixed after --> all the changes (and unit test) is submitted in 'svn diff' format --> you verbally assure me that you ran 'ant test' and everything passed. Regarding the diff format-- it's easy. Just checkout the latest trunk code from subversion. Make your changes. Go to the root directory and type "svn diff" and capture the output. That should capture in one file the code change and the unit test change. If you don't have time to do those things, we still appreciate your effort, but it may be a while until I have time to work through the issue and verify the patch works but doesn't break anything else.
        Hide
        Etienne Massip added a comment - - edited

        here is the patch.

        Met another problem writting my test case, unrelated to the current issue :

        It seems that there is not eol characters in output when the syntax is :

        #foreach(..)
        my output whith #evaluate(..)
        #end

        gives on a single line :

        my output whith my output whith etc...

        Eol is written when syntax is :

        #foreach(..)
        my output whith #evaluate(..)1
        #end

        gives as expected a multiline output :

        my output whith (?)1
        my output whith (?)1
        etc...

        so my test case does not expect any eol (in .cmp).
        seems like parser is responsible for not creating an ASTText node containing the \r\n sequence.

        hope it helps

        Show
        Etienne Massip added a comment - - edited here is the patch. Met another problem writting my test case, unrelated to the current issue : It seems that there is not eol characters in output when the syntax is : #foreach(..) my output whith #evaluate(..) #end gives on a single line : my output whith my output whith etc... Eol is written when syntax is : #foreach(..) my output whith #evaluate(..)1 #end gives as expected a multiline output : my output whith (?)1 my output whith (?)1 etc... so my test case does not expect any eol (in .cmp). seems like parser is responsible for not creating an ASTText node containing the \r\n sequence. hope it helps
        Hide
        Will Glass-Husain added a comment -

        Nicely done. Can you please confirm "ant test" passes?

        The eol behavior is expected. Directives with no text following swallow the newline. This may seem a little odd here but makes more sense for #foreach , #end, etc.

        Show
        Will Glass-Husain added a comment - Nicely done. Can you please confirm "ant test" passes? The eol behavior is expected. Directives with no text following swallow the newline. This may seem a little odd here but makes more sense for #foreach , #end, etc.
        Hide
        Etienne Massip added a comment -

        I do confirm.

        Show
        Etienne Massip added a comment - I do confirm.
        Hide
        Etienne Massip added a comment -

        Please, could you tell us when you plan to commit this patch, I mean if it is okay for you ?

        Show
        Etienne Massip added a comment - Please, could you tell us when you plan to commit this patch, I mean if it is okay for you ?
        Hide
        Will Glass-Husain added a comment -

        oops - sorry - forgot about this. yes, will do so tomorrow.

        WILL

        Show
        Will Glass-Husain added a comment - oops - sorry - forgot about this. yes, will do so tomorrow. WILL
        Hide
        Will Glass-Husain added a comment -

        patch committed. sorry about the delay to check this in. thanks again.

        Show
        Will Glass-Husain added a comment - patch committed. sorry about the delay to check this in. thanks again.

          People

          • Assignee:
            Will Glass-Husain
            Reporter:
            Will Glass-Husain
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development