From 416d98a25fc693f7ef8493c29cf91c2df729a0b9 Mon Sep 17 00:00:00 2001 From: Peter Menhart Date: Wed, 24 Apr 2024 01:58:57 -0400 Subject: [PATCH] Fix rendering of custom tags with more than one parameter (#301) * 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"] --- ruby/cases_render_tag.rb | 38 +++++++++++++++++++ src/main/java/liqp/parser/v4/NodeVisitor.java | 6 ++- src/test/java/liqp/InsertionTest.java | 18 +++++++++ .../java/liqp/parser/v4/LiquidParserTest.java | 13 +++++++ 4 files changed, 73 insertions(+), 2 deletions(-) create mode 100755 ruby/cases_render_tag.rb diff --git a/ruby/cases_render_tag.rb b/ruby/cases_render_tag.rb new file mode 100755 index 00000000..89917370 --- /dev/null +++ b/ruby/cases_render_tag.rb @@ -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 + diff --git a/src/main/java/liqp/parser/v4/NodeVisitor.java b/src/main/java/liqp/parser/v4/NodeVisitor.java index 2b45ab27..f5abbcf9 100644 --- a/src/main/java/liqp/parser/v4/NodeVisitor.java +++ b/src/main/java/liqp/parser/v4/NodeVisitor.java @@ -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; @@ -194,9 +195,10 @@ public LNode visitSimple_tag(Simple_tagContext ctx) { List expressions = new ArrayList(); 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()])); } diff --git a/src/test/java/liqp/InsertionTest.java b/src/test/java/liqp/InsertionTest.java index 9bd4dc91..34d31534 100644 --- a/src/test/java/liqp/InsertionTest.java +++ b/src/test/java/liqp/InsertionTest.java @@ -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") { diff --git a/src/test/java/liqp/parser/v4/LiquidParserTest.java b/src/test/java/liqp/parser/v4/LiquidParserTest.java index 964fbc4d..75d5d8b9 100644 --- a/src/test/java/liqp/parser/v4/LiquidParserTest.java +++ b/src/test/java/liqp/parser/v4/LiquidParserTest.java @@ -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", "%}"))