Skip to content

Commit

Permalink
Fix rendering of custom tags with more than one parameter (#301)
Browse files Browse the repository at this point in the history
* Improved parsing of flavor-specific tag 'include_relative'

* In Jekyll style, 'include_relative' was represented as token SimpleTag,
  instead of IncludeRelative, as expected since PR #288.
* As a consequence, filename components were parsed as separate parameters
  instead of token file_name_or_output. This behaviour got unnoticed only
  because all parameters were incorrectly concatenated into one string
  (to be fixed outside of this commit)
* In Liquid style, tag 'include_relative' is not defined (token InvalidTagId),
  but can be defined as a custom block (token SimpleBlock) or a tag (SimpleTag).
* To facilitate flavor-specific lexical analysis, flag isLiquidStyleInclude was
  was added to LiquidLexer; same code as in the LiquidParser since #196

* Fixed custom tags with more than one parameter

* All tag parameters were concatenated into one string,
  e.g. {% mu for foo as bar %} was presented as one node ["forfooasbar"] in Tag method render()
* Caused by combining text of all childen of token 'other_tag_parameters' in NodeVisitor.visitSimple_tag().
  Change to one string in #227 seems to be not intentional; reverting to a list
* This commit separates each parameter to a distinct node, i.e. ["for", "foo", "as", "bar"]
  • Loading branch information
pmenhart authored Apr 24, 2024
1 parent 085001d commit 416d98a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 2 deletions.
38 changes: 38 additions & 0 deletions ruby/cases_render_tag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/usr/bin/env ruby

require 'pp'
require_relative '_helpers.rb'

# pp render({"val" => "here"}, "{{ val }}")

if isJekyll
context = jekyllContext("includes_dir" => "cases_variable_inside_import/_includes")
# Tag 'render' is not defined in Jekyll. Use 'include' or 'include_relative'
assertRaise do
assertEqual("TEST", render({}, "{% render color.liquid %}", context))
end
else
Liquid::Template.file_system = Liquid::LocalFileSystem.new("cases_variable_inside_import/_includes", "%s.liquid")

# liquid template name must be a quoted string
assertRaise do
assertEqual("there", render({"var" => "there", "tmpl" => "include_read_var"}, "{% render tmpl %}"))
end
# render variables
assertEqual("", render({"var" => "there"}, "{% render 'include_read_var' %}"))
assertEqual("here", render({"var" => "there"}, "{% render 'include_read_var', var: 'here' %}"))
assertEqual("there", render({"var" => "there"}, "{% render 'include_read_var', var: var %}"))
assertEqual("color: ''", render({"color" => "red"}, "{% render 'color' %}"))
assertEqual("color: 'blue'", render({"color" => "red"}, "{% render 'color', color: 'blue' %}"))
assertEqual("color: 'red'", render({"color" => "red"}, "{% render 'color', color: color %}"))
# render with
assertEqual("color: 'red'", render({"var" => "there"}, "{% render 'color' with 'red' %}"))
assertEqual("color: 'red'", render({"var" => "there"}, "{% render 'color' with 'red', color: 'blue' %}"))
assertEqual("color: 'blue'", render({"clr" => "blue"}, "{% render 'color' with clr %}"))
assertEqual("color: 'blue'", render({"clr" => "blue"}, "{% render 'color' with clr, color: 'green' %}"))
# render for
assertEqual("color: 'r'color: 'g'color: 'b'", render({"colors" => ["r", "g", "b"]}, "{% render 'color' for colors %}"))
assertEqual("rgb", render({"colors" => ["r", "g", "b"]}, "{% render 'include_read_var' for colors as var %}"))
assertEqual("foofoofoo", render({"colors" => ["r", "g", "b"]}, "{% render 'include_read_var' for colors as clr, var: 'foo' %}"))
end

6 changes: 4 additions & 2 deletions src/main/java/liqp/parser/v4/NodeVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import liquid.parser.v4.LiquidParser;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.misc.Interval;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNode;

import liqp.Insertion;
Expand Down Expand Up @@ -194,9 +195,10 @@ public LNode visitSimple_tag(Simple_tagContext ctx) {
List<LNode> expressions = new ArrayList<LNode>();

if (ctx.other_tag_parameters() != null) {
expressions.add(new AtomNode(ctx.other_tag_parameters().getText()));
for (ParseTree child : ctx.other_tag_parameters().other_than_tag_end().children) {
expressions.add(new AtomNode(child.getText()));
}
}

return new InsertionNode(insertions.get(ctx.SimpleTagId().getText()), expressions.toArray(new LNode[expressions.size()]));
}

Expand Down
18 changes: 18 additions & 0 deletions src/test/java/liqp/InsertionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,24 @@ public Object render(TemplateContext context, LNode... nodes) {
assertThat(rendered, is("20.0"));
}

@Test
public void testCustomTagParameters() throws RecognitionException {

TemplateParser parser = new TemplateParser.Builder().withTag(new Tag("multiply") {
@Override
public Object render(TemplateContext context, LNode... nodes) {
Double number1 = super.asNumber(nodes[0].render(context)).doubleValue();
Double number2 = super.asNumber(nodes[1].render(context)).doubleValue();
return number1 * number2;
}
}).build();

Template template = parser.parse("{% multiply 2 4 %}");
String rendered = String.valueOf(template.render());

assertThat(rendered, is("8.0"));
}

@Test
public void testCustomTagBlock() throws RecognitionException {
TemplateParser templateParser = new TemplateParser.Builder().withBlock(new Block("twice") {
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/liqp/parser/v4/LiquidParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ public void testCustom_tag() {
equalTo(array("{%", "mu", "|42", "%}"))
);

assertThat(
"tag parameters are concatenated into one String by texts()",
texts("{% mu for foo as bar %}", "simple_tag", emptySet, muSet),
equalTo(array("{%", "mu", "forfooasbar", "%}"))
);

ParseTree tree = parse("{% mu for foo as bar %}", "simple_tag", emptySet, muSet);
assertThat(
"tag parameters are parsed as separate nodes",
texts(tree.getChild(2).getChild(0)),
equalTo(array("for", "foo", "as", "bar"))
);

assertThat(
texts("{% mu %} . {% endmu %}", "other_tag", muSet, emptySet),
equalTo(array("{%", "mu", "%}", " . ", "{%", "endmu", "%}"))
Expand Down

0 comments on commit 416d98a

Please sign in to comment.