Velocity
  1. Velocity
  2. VELOCITY-362

can't load macros in file loaded with #parse

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.6
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All

      Description

      I think this is a big bug. I've used velocity in my projects, erveything is ok
      but this. i want to know it's this problem will be resolved in next version? if
      it's not, i have to abandon velocity.
      the bug is :
      from doc:
      This is important to remember if you try to #parse() a template containing
      inline #macro() directives. Because the #parse() happens at runtime, and the
      parser decides if a VM-looking element in the template is a VM at parsetime,
      #parse()-ing a set of VM declarations won't work as expected. To get around
      this, simply use the velocimacro.library facility to have Velocity load your
      VMs at startup.

      1. patch.zip
        23 kB
        Supun Kamburugamuva
      2. small.patch
        0.7 kB
        Supun Kamburugamuva

        Issue Links

          Activity

          Hide
          Will Glass-Husain added a comment -

          Let's put this on the list for v1.5

          Show
          Will Glass-Husain added a comment - Let's put this on the list for v1.5
          Hide
          Will Glass-Husain added a comment -

          This is trickier than it seems. See my thoughts at http://wiki.apache.org/jakarta-velocity/MacroIssues
          Moving to v1.6

          Show
          Will Glass-Husain added a comment - This is trickier than it seems. See my thoughts at http://wiki.apache.org/jakarta-velocity/MacroIssues Moving to v1.6
          Hide
          Nathan Bubna added a comment -

          Since this is documented behavior, it's not really appropriate to call it a bug.

          Show
          Nathan Bubna added a comment - Since this is documented behavior, it's not really appropriate to call it a bug.
          Hide
          Henning Schmiedehausen added a comment -

          As we already have discussed that issue in length; as it is on our Macro wishlist, there is no need to keep the report open. The reported bug is the documented behaviour; yes it is not really nice but sometimes there is no pony. Sorry 'bout that.

          Show
          Henning Schmiedehausen added a comment - As we already have discussed that issue in length; as it is on our Macro wishlist, there is no need to keep the report open. The reported bug is the documented behaviour; yes it is not really nice but sometimes there is no pony. Sorry 'bout that.
          Hide
          Will Glass-Husain added a comment -

          As discussed on the dev list, I see this as an important "to-do" for version 1.6. Reopening.

          Show
          Will Glass-Husain added a comment - As discussed on the dev list, I see this as an important "to-do" for version 1.6. Reopening.
          Hide
          Supun Kamburugamuva added a comment -

          I have attached a zip file with a patch. This patch solves both VELOCITY-529 and VELOCITY-362. I have added two new folders with test files to the code. But it seems like I cannot create a patch with these new folders. So I have created a zip file with these new folders and here is the structure of the zip content.

          -final. patch: a patch with the added folders.
          If this works we only need the above patch file.

          If the above patch cannot be applied we can use the following patch. After the patch is applied we need to copy the two folders in the zip file in to "test" files folder

          -final_without_folders.patch: a patch without the added folders
          -macrolibs and parsemacros: these the two folders required.

          Another issue is that I got a SVN warning about inconsistent line endings in JavaCC generated files. So I did include only the parser.jjt with the changes. To use the patch, parser should be recompiled using JavaCC.

          Show
          Supun Kamburugamuva added a comment - I have attached a zip file with a patch. This patch solves both VELOCITY-529 and VELOCITY-362 . I have added two new folders with test files to the code. But it seems like I cannot create a patch with these new folders. So I have created a zip file with these new folders and here is the structure of the zip content. -final. patch: a patch with the added folders. If this works we only need the above patch file. If the above patch cannot be applied we can use the following patch. After the patch is applied we need to copy the two folders in the zip file in to "test" files folder -final_without_folders.patch: a patch without the added folders -macrolibs and parsemacros: these the two folders required. Another issue is that I got a SVN warning about inconsistent line endings in JavaCC generated files. So I did include only the parser.jjt with the changes. To use the patch, parser should be recompiled using JavaCC.
          Hide
          Will Glass-Husain added a comment -

          This is really a great patch. Nice enhancement to the macro functionality. An easily understandable structure. Good code style. The patch file worked fine for me (Eclipse automatically creates the new file).

          I made one minor change to the code – in mergeTemplate I changed the catch(Exception) clause to rethrow a RuntimeException instead of logging it. I also made minor changes to some of the comments.

          One big problem. "ant test" does not pass. The parsing of the new test templates does not match the compare files.

          Could you have submitted the wrong test or compare templates? (or could they be different in the patch file)?

          I

          Show
          Will Glass-Husain added a comment - This is really a great patch. Nice enhancement to the macro functionality. An easily understandable structure. Good code style. The patch file worked fine for me (Eclipse automatically creates the new file). I made one minor change to the code – in mergeTemplate I changed the catch(Exception) clause to rethrow a RuntimeException instead of logging it. I also made minor changes to some of the comments. One big problem. "ant test" does not pass. The parsing of the new test templates does not match the compare files. Could you have submitted the wrong test or compare templates? (or could they be different in the patch file)? I
          Hide
          Will Glass-Husain added a comment -

          Hi,

          Supun and I had a chance to do a little IM. It turns out the fault is on my end . I'd improperly loaded the test files. The patch is perfect. ant test passes fine.

          I want to study the test cases a little more, than I'll commit. Supun has promised to discuss the improvements in more detail on the dev list. Again, very nice work.

          Show
          Will Glass-Husain added a comment - Hi, Supun and I had a chance to do a little IM. It turns out the fault is on my end . I'd improperly loaded the test files. The patch is perfect. ant test passes fine. I want to study the test cases a little more, than I'll commit. Supun has promised to discuss the improvements in more detail on the dev list. Again, very nice work.
          Hide
          Will Glass-Husain added a comment -

          Patch applied. I refactored the tests a bit to test all 4 combinations of cache on/off and local/global macro namespace. the tests also check that if you change the #parse'd file, the macro definitions change.

          I'm going to leave this issue open – there's a few changes to the docs that are needed. We can close the issue once the docs are updated.

          Show
          Will Glass-Husain added a comment - Patch applied. I refactored the tests a bit to test all 4 combinations of cache on/off and local/global macro namespace. the tests also check that if you change the #parse'd file, the macro definitions change. I'm going to leave this issue open – there's a few changes to the docs that are needed. We can close the issue once the docs are updated.
          Hide
          Supun Kamburugamuva added a comment -

          There is a lack of a feature in the code due to a small mistake. That
          was when a user includes two macro libraries via the #parse directive,
          if the libraries contain the same macro, the library included later
          should take precedence. But it was not supported and it was due to a
          very simple mistake (for loop counting upwards instead downwards). I
          have corrected this and attached a patch containing the changes.

          Show
          Supun Kamburugamuva added a comment - There is a lack of a feature in the code due to a small mistake. That was when a user includes two macro libraries via the #parse directive, if the libraries contain the same macro, the library included later should take precedence. But it was not supported and it was due to a very simple mistake (for loop counting upwards instead downwards). I have corrected this and attached a patch containing the changes.
          Hide
          Will Glass-Husain added a comment -

          Thanks. a test case would be useful here - this is a subtle but important behavior; it'd be useful to check it.

          Show
          Will Glass-Husain added a comment - Thanks. a test case would be useful here - this is a subtle but important behavior; it'd be useful to check it.
          Hide
          Will Glass-Husain added a comment -

          added final small patch, with test case. fixed docs (docbook version of user guide).

          Show
          Will Glass-Husain added a comment - added final small patch, with test case. fixed docs (docbook version of user guide).
          Hide
          Tim White added a comment -

          A huge thank you to Supun for this patch to an issue that I'd reported long ago. Thanks to Will and Nathan for getting it in to 1.6!

          Show
          Tim White added a comment - A huge thank you to Supun for this patch to an issue that I'd reported long ago. Thanks to Will and Nathan for getting it in to 1.6!

            People

            • Assignee:
              Unassigned
              Reporter:
              whxbb
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development