Shopify / liquid

Liquid markup language. Safe, customer facing template language for flexible web apps.
https://shopify.github.io/liquid/
MIT License
11.05k stars 1.38k forks source link

fix parsing comment tag with extra string #1769

Closed ggmichaelgo closed 9 months ago

ggmichaelgo commented 9 months ago

What are you trying to solve?

https://github.com/Shopify/liquid/pull/1755 introduced a bug in comment tag that prevents the tag from having extra string inside.

With the bug, this valid template fails to be parsed

{%comment}{% assign a = 1 %}
  {% if true %}
{%endcomment}{% endif %}
peterzhu2118 commented 9 months ago

I'm confused about the example template. Is {%comment} valid opening for a comment? Is {%endcomment} a valid closing? What's the {% endif %} supposed to close?

ggmichaelgo commented 9 months ago

I'm confused about the example template. Is {%comment} valid opening for a comment? Is {%endcomment} a valid closing? What's the {% endif %} supposed to close?

{%endcomment}{% endif %} is the comment tag delimiter.

With Liquid 5.4.0, you can see how the comment tag used to get parsed.

require 'liquid'

template = Liquid::Template.parse("{%comment}{% assign a = 1 %}{%endcomment}{% endif %}")
pp template.root.nodelist[0]
ggmichaelgo commented 9 months ago

I agree that example Liquid template is awful to read, and I wish we could deprecate some Liquid syntaxes... However, we can't deprecate this syntax pattern on Liquid because we can't break existing Shopify themes...