Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • 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 am using VTL (with VPP) to customize a portion of a Perl script. Since Perl
      code has a lot of dollar signs, I am using #literal()/#end to prevent Velocity
      from processing most of the file. What I find is that single '#' characters
      that are not followed by alpha text are removed (inside #literal()/#end).

      For example:

      #literal()
      #!/usr/bin/perl
      #end

      becomes:

      !/usr/bin/perl

      I've tried things like escaping the '#' ('#') but that leaves the backslash
      ('#!/usr/bin/perl' becomes '!/usr/bin/perl'). Nothing seems to work.

      I can use a #set to define a variable with the value '#!/usr/bin/perl', but I
      was hoping I would not have to. It also doesn't help with other single #'s in
      the file (like Perl comments). For those I have had to double up the hashes.

      I'd be happy to try patching the Velocity source, but I had trouble making heads
      or tails of the parser engine?

        Activity

        Hide
        Will Glass-Husain added a comment -

        Acknowledged - thanks.

        I'm impressed you found this. It seems to be an undocumented language
        feature. As such it's not surprising that it's not polished.

        For the record, the code for this is
        org.apache.velocity.runtime.directive.Literal

        To fix this, you won't need to change the Parser.* code, just change
        Literal.java. You'll want to run a debugger and see what the children are for
        node. My guess is that this line:

        literalText = node.jjtGetChild(0).literal();

        needs to loop through all the children and concatenate the text instead of
        just getting the text for the first node.

        Show
        Will Glass-Husain added a comment - Acknowledged - thanks. I'm impressed you found this. It seems to be an undocumented language feature. As such it's not surprising that it's not polished. For the record, the code for this is org.apache.velocity.runtime.directive.Literal To fix this, you won't need to change the Parser.* code, just change Literal.java. You'll want to run a debugger and see what the children are for node. My guess is that this line: literalText = node.jjtGetChild(0).literal(); needs to loop through all the children and concatenate the text instead of just getting the text for the first node.
        Hide
        Geoffrey Lowney added a comment -

        (In reply to comment #1)
        > To fix this, you won't need to change the Parser.* code, just change
        > Literal.java. You'll want to run a debugger and see what the children are for
        > node. My guess is that this line:
        >
        > literalText = node.jjtGetChild(0).literal();
        >
        > needs to loop through all the children and concatenate the text instead of
        > just getting the text for the first node.

        This doesn't seem to work. I added the loop you suggest as follows (with some
        debugging statements):

        StringBuffer literalTextBuffer = new StringBuffer();
        for (int i = 0; i < node.jjtGetNumChildren(); i++)

        { literalTextBuffer.append(node.jjtGetChild(i).literal()); System.out.println("literalTextBuffer=["+literalTextBuffer+"]"); }

        literalText = literalTextBuffer.toString();
        System.out.println("literalText=["+literalText+"]");

        I also updated the literal.vm test files to check if the change works. I added
        a line "#!/bin/sh" (a line containing a single "#" followed by non-alpha text)
        near the end of the #literal/#end block. What I can see when I run the test is
        that there is only one child node. The one child node contains the entire block
        of text from #literal to #end (not including #literal/#end), and the "#" in
        front of "!/bin/sh" has alredy been removed.

        [java] Adding TemplateTestCase : settest
        [java] ...............................literalTextBuffer=[
        [java] #foreach ($woogie in $boogie)
        [java] nothing will happen to $woogie
        [java] #end

        [java] #if($skin)
        [java] $!data.setLayoutTemplate($!skin.getLayout())
        [java] $!page.setCss($!skin.getCss())
        [java] #end

        [java] !/bin/sh

        [java] ]
        [java] literalText=[
        [java] #foreach ($woogie in $boogie)
        [java] nothing will happen to $woogie
        [java] #end

        [java] #if($skin)
        [java] $!data.setLayoutTemplate($!skin.getLayout())
        [java] $!page.setCss($!skin.getCss())
        [java] #end

        [java] !/bin/sh

        [java] ]
        [java] F........
        [java] Time: 1.758
        [java] There was 1 failure:
        [java] 1)
        Literal(org.apache.velocity.test.TemplateTestCase)junit.framework.AssertionFailedError:
        Processed template did not match expected output
        [java] at
        org.apache.velocity.test.TemplateTestCase.runTest(TemplateTestCase.java:192)

        Show
        Geoffrey Lowney added a comment - (In reply to comment #1) > To fix this, you won't need to change the Parser.* code, just change > Literal.java. You'll want to run a debugger and see what the children are for > node. My guess is that this line: > > literalText = node.jjtGetChild(0).literal(); > > needs to loop through all the children and concatenate the text instead of > just getting the text for the first node. This doesn't seem to work. I added the loop you suggest as follows (with some debugging statements): StringBuffer literalTextBuffer = new StringBuffer(); for (int i = 0; i < node.jjtGetNumChildren(); i++) { literalTextBuffer.append(node.jjtGetChild(i).literal()); System.out.println("literalTextBuffer=["+literalTextBuffer+"]"); } literalText = literalTextBuffer.toString(); System.out.println("literalText= ["+literalText+"] "); I also updated the literal.vm test files to check if the change works. I added a line "#!/bin/sh" (a line containing a single "#" followed by non-alpha text) near the end of the #literal/#end block. What I can see when I run the test is that there is only one child node. The one child node contains the entire block of text from #literal to #end (not including #literal/#end), and the "#" in front of "!/bin/sh" has alredy been removed. [java] Adding TemplateTestCase : settest [java] ...............................literalTextBuffer=[ [java] #foreach ($woogie in $boogie) [java] nothing will happen to $woogie [java] #end [java] #if($skin) [java] $!data.setLayoutTemplate($!skin.getLayout()) [java] $!page.setCss($!skin.getCss()) [java] #end [java] !/bin/sh [java] ] [java] literalText=[ [java] #foreach ($woogie in $boogie) [java] nothing will happen to $woogie [java] #end [java] #if($skin) [java] $!data.setLayoutTemplate($!skin.getLayout()) [java] $!page.setCss($!skin.getCss()) [java] #end [java] !/bin/sh [java] ] [java] F........ [java] Time: 1.758 [java] There was 1 failure: [java] 1) Literal(org.apache.velocity.test.TemplateTestCase)junit.framework.AssertionFailedError: Processed template did not match expected output [java] at org.apache.velocity.test.TemplateTestCase.runTest(TemplateTestCase.java:192)
        Hide
        Will Glass-Husain added a comment -

        ah well, so much for that theory. I'll take a look at this when I have time,
        unless someone else wants to dig in first.

        Show
        Will Glass-Husain added a comment - ah well, so much for that theory. I'll take a look at this when I have time, unless someone else wants to dig in first.
        Hide
        Henning Schmiedehausen added a comment -

        This is one of the quirks in the current parser. Yes, it sucks. However, I'm not sure that we can amend that behaviour short of a complete parser rewrite. For that I postpone it to 2.0.

        Show
        Henning Schmiedehausen added a comment - This is one of the quirks in the current parser. Yes, it sucks. However, I'm not sure that we can amend that behaviour short of a complete parser rewrite. For that I postpone it to 2.0.
        Hide
        Nathan Bubna added a comment -

        Not really a parser issue. The missing chars were special tokens and SimpleNode.literal() was not using NodeUtils.tokenLiteral() to get those back.

        Show
        Nathan Bubna added a comment - Not really a parser issue. The missing chars were special tokens and SimpleNode.literal() was not using NodeUtils.tokenLiteral() to get those back.
        Hide
        Jarkko Viinamäki added a comment -

        Nathan have you checked if this change has any significant impact on performance?

        NodeUtils.tokenLiteral looks a bit heavy and since it is now called in SimpleNode, it is used in many parsed nodes. If the bug is related to only #literal directive and there's a performance penalty, would it be possible to modify just Literal.init instead like Will suggested? The fix works if we use a modified literal() method in Literal class.

        Here's a small patch I used to test this.

        Show
        Jarkko Viinamäki added a comment - Nathan have you checked if this change has any significant impact on performance? NodeUtils.tokenLiteral looks a bit heavy and since it is now called in SimpleNode, it is used in many parsed nodes. If the bug is related to only #literal directive and there's a performance penalty, would it be possible to modify just Literal.init instead like Will suggested? The fix works if we use a modified literal() method in Literal class. Here's a small patch I used to test this.
        Hide
        Nathan Bubna added a comment -

        Yeah, i ran your testbench and didn't see any penalty. But i spruced up NodeUtils.tokenLiteral's performance a bit anyway.

        I did think about just tweaking Literal.init, except that doesn't fix the fact SimpleNode.literal() isn't doing the right thing. Also, so far as i can tell, NodeUtils.tokenLiteral() is only "heavy" under circumstances which require it, especially with the improvements i just added. So, while fixing this in #literal would solve the apparent problems, that still leaves risk of SimpleNode.literal() showing problems in other circumstances.

        Show
        Nathan Bubna added a comment - Yeah, i ran your testbench and didn't see any penalty. But i spruced up NodeUtils.tokenLiteral's performance a bit anyway. I did think about just tweaking Literal.init, except that doesn't fix the fact SimpleNode.literal() isn't doing the right thing. Also, so far as i can tell, NodeUtils.tokenLiteral() is only "heavy" under circumstances which require it, especially with the improvements i just added. So, while fixing this in #literal would solve the apparent problems, that still leaves risk of SimpleNode.literal() showing problems in other circumstances.

          People

          • Assignee:
            Unassigned
            Reporter:
            Geoffrey Lowney
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development