Velocity
  1. Velocity
  2. VELOCITY-686

BlockMacro renders $bodyContent on #set

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7
    • Component/s: Engine
    • Labels:
      None

      Description

      Given the following VTL:

      #macro(foo)#set($x = $bodyContent)#end
      #@foo()bar#end

      renders to:

      bar

      I wonder about calling render on JJTBLOCK types in the ProxyVMContext get method. Do you think it would be better in ASTReference render?

      There is an interesting use case for BlockMacros which looks something like this:

      #macro(escXML)$tool.escXml($bodyContent)#end

      The above definition could be used to create a filter like the following:

      #@escXML

        1. ... rendered stuff to be escaped
          #end

      $tool.escXml can intercept the writer and create a filter. But as it stands referencing $bodyContent renders the content making this use case impractical.

      1. velocity-686.patch
        12 kB
        Jarkko Viinamäki

        Activity

        Hide
        Jarkko Viinamäki added a comment -

        This is not actually a bug. It's a design decision. $bodyContent is not a normal reference because it can contain any complex set of AST nodes and logic. It just marks where the passed body should be rendered. Therefore you can't use it like a normal reference.

        Show
        Jarkko Viinamäki added a comment - This is not actually a bug. It's a design decision. $bodyContent is not a normal reference because it can contain any complex set of AST nodes and logic. It just marks where the passed body should be rendered. Therefore you can't use it like a normal reference.
        Hide
        Byron Foster added a comment -

        Looking at the Define directive implementation, it seems to handle this pretty well.

        Show
        Byron Foster added a comment - Looking at the Define directive implementation, it seems to handle this pretty well.
        Hide
        Nathan Bubna added a comment -

        I'm with Byron. I think this ought to work like a block created with #define.

        Show
        Nathan Bubna added a comment - I'm with Byron. I think this ought to work like a block created with #define.
        Hide
        Jarkko Viinamäki added a comment -

        I'll see what I can do to refactor this.

        Show
        Jarkko Viinamäki added a comment - I'll see what I can do to refactor this.
        Hide
        Jarkko Viinamäki added a comment -

        OK. I took a look at Define and noticed that Renderable interface is very helpful since it's already inside ASTReference logic. As a result I was able to refactor BlockMacro implementation to simpler format.

        This should also fix the bug and $bodyContent reference should be usable like reference defined by #define.

        Show
        Jarkko Viinamäki added a comment - OK. I took a look at Define and noticed that Renderable interface is very helpful since it's already inside ASTReference logic. As a result I was able to refactor BlockMacro implementation to simpler format. This should also fix the bug and $bodyContent reference should be usable like reference defined by #define.
        Hide
        Nathan Bubna added a comment -

        Curse my semi-offline semi-vacation! I was bored and away from wifi last night and took a stab at this myself, only to find you did too. I hate duplicating effort, but oh well. We approached it much the same, expect that i kept $bodyContent as a proxy arg rather than put it in the context, because i was concerned about the effect on nesting body macros. I also consolidated some duplicated stuff from Define. I hope you don't mind, but i currently prefer my implementation. I'll check it in shortly. Please let me know what you think of it.

        Show
        Nathan Bubna added a comment - Curse my semi-offline semi-vacation! I was bored and away from wifi last night and took a stab at this myself, only to find you did too. I hate duplicating effort, but oh well. We approached it much the same, expect that i kept $bodyContent as a proxy arg rather than put it in the context, because i was concerned about the effect on nesting body macros. I also consolidated some duplicated stuff from Define. I hope you don't mind, but i currently prefer my implementation. I'll check it in shortly. Please let me know what you think of it.
        Hide
        Jarkko Viinamäki added a comment -

        That's cool Nathan. I always prefer the best implementation - whoever happens to provide it. So I'm glad you found even better solution.

        Show
        Jarkko Viinamäki added a comment - That's cool Nathan. I always prefer the best implementation - whoever happens to provide it. So I'm glad you found even better solution.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development