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

        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?
        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.
        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.
        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
        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.

          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