Velocity
  1. Velocity
  2. VELOCITY-459

localscope doesn't allow get to leak

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5
    • Fix Version/s: 1.5
    • Component/s: Engine
    • Labels:
      None

      Description

      In short, we wanted "localscope" on for macros due to some recursion. However, this meant that both puts /and/ gets only use the localscope.

      For gets this seems overly restrictive--if I set "foo=bar" in my base VelocityContext, and then get into my macro, it should be able to reach that "foo" even if it can't overwrite it.

      The attached patch relaxes the "VMContext.get" to let it check first the local scope and then the inner scope. Included is a test that makes sure an "outsideVar" is accessible from VMContext even if localscope is on.

      1. localscopeGetFix.diff
        4 kB
        Stephen Haberman

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        2d 17h 34m 1 Will Glass-Husain 24/Sep/06 00:40
        Resolved Resolved Closed Closed
        165d 24m 1 Henning Schmiedehausen 08/Mar/07 00:04
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12551370 ] jira [ 12552078 ]
        Mark Thomas made changes -
        Workflow jira [ 12384166 ] Default workflow, editable Closed status [ 12551370 ]
        Henning Schmiedehausen made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Henning Schmiedehausen added a comment -

        Close all resolved issues for Engine 1.5 release.

        Show
        Henning Schmiedehausen added a comment - Close all resolved issues for Engine 1.5 release.
        Will Glass-Husain made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hide
        Will Glass-Husain added a comment -

        patch applied. thanks again. revision #449333.

        Show
        Will Glass-Husain added a comment - patch applied. thanks again. revision #449333.
        Hide
        Will Glass-Husain added a comment -

        Reviewed the patch. Looks fine to me. (Minor formatting issues with the braces in the unit test). Will apply.

        Show
        Will Glass-Husain added a comment - Reviewed the patch. Looks fine to me. (Minor formatting issues with the braces in the unit test). Will apply.
        Will Glass-Husain made changes -
        Assignee Will Glass-Husain [ wglass ]
        Hide
        Stephen Haberman added a comment -

        Yep, "ant test" passes. Thanks for making it easy to run the tests (e.g. I didn't have to install maven--whew.)

        No, I do not think it is related to 262--that seems to be talking about a parsing issue. I guess.

        For us, everything parsed fine, we just kept getting back as "null" any variables defined outside our variable. So we wanted to turn localscope off. But then we'd do macro recursion and they'd stomp on each others internal variables when setting, and we'd want localscope back on. This let us have the best of both worlds.

        Show
        Stephen Haberman added a comment - Yep, "ant test" passes. Thanks for making it easy to run the tests (e.g. I didn't have to install maven--whew.) No, I do not think it is related to 262--that seems to be talking about a parsing issue. I guess. For us, everything parsed fine, we just kept getting back as "null" any variables defined outside our variable. So we wanted to turn localscope off. But then we'd do macro recursion and they'd stomp on each others internal variables when setting, and we'd want localscope back on. This let us have the best of both worlds.
        Will Glass-Husain made changes -
        Priority Major [ 3 ] Minor [ 4 ]
        Fix Version/s 1.5 [ 12310253 ]
        Hide
        Will Glass-Husain added a comment -

        Thanks for reporting this and submitting a patch. Does "ant test" pass? We should get this into 1.5.

        Is this related to Velocity-262?

        Show
        Will Glass-Husain added a comment - Thanks for reporting this and submitting a patch. Does "ant test" pass? We should get this into 1.5. Is this related to Velocity-262?
        Stephen Haberman made changes -
        Field Original Value New Value
        Attachment localscopeGetFix.diff [ 12341251 ]
        Stephen Haberman created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development