Request for suggestions: Liquid comments

[tobi]

Right now comments in liquid are implemented through the {% comment %} tag. Basically a block that doesn't do anything.

This has always been clunky and I think we should introduce syntax for this.

This is especially true because of the new liquid block tag. Consider that this is the only way to do inline comments right now:

{% liquid
comment Is the feature_image set? 
endcomment

if product.featured_image
  echo product.featured_image | img_tag
else
  echo 'product-1' 
endif 
%}

Which... is bad.

My current thinking is that we should double down on our ruby heritage and designate # as the comment character. Alternatives here would be c style // or lua style --

This would transform the above to

{% liquid
# Is the feature_image set? 
if product.featured_image
  echo product.featured_image | img_tag
else
  echo 'product-1' 
endif 
%}

Additionally we could introduce a new inline token for comments:
Single line comment {# comment #} and multiline comments would be both supported

{# 
  First line 
  Second line
#}

The downside of this syntax is that it may cause collisions in existing liquid code. We can (but haven't) check the Shopify databases for how common this might be.

[samdoiron]

There are some advantages in consistency if we define the # character (or //, -- etc) as a standard Liquid tag. This would require only a small change to the core grammar, and would give us most of the other things we would want for free.

Examples:

{% # using a wrapper element for IE compatibility %}
<div class="foo">
  ...

{% liquid
# Support for the Liquid tag comes for free, since comments are just
# standard tag calls. 
if product.featured_image
  echo product.featured_image | img_tag
else
  echo 'product-1' 
endif 
%}

{% # this sort of comment also
   # will just work, because it parses
   # as a single call to the "#" tag %}

[tobi]

OK that makes perfect sense and means that we don't need a new tag at all.

The one question is whitespace management. I'd like for comments to imply {%- -%} whitespace eating. Perhaps we can just introduce (or do we already do that?) a rule that a tag that doesn't output anything itself always eats surrounding whitespace. I imagine this would change resulting HTML, especially with common if and for tags. But... that's probably good?

HTML doesn't care much about whitespace but plain text does. Many use cases of liquid exist around plain text, so if we change the default we definitely need a fallback.

[resistorsoftware]

In my long life of writing code, I have almost never run into trouble using the hash # symbol to introduce a comment, but I have had a lot of trouble with the double-slash // and its block companion /* */ so I am all over this introduction of a single tag symbol to open a comment. Also, the new liquid block is sexy. Super nice and also somewhat amazing that it took so long to evolve! You know it is right when it just feels right and not forced.

[ashukasma]

I would prefer to use # because the same work with shopify scripts, more towards ruby comments.

single line comment should work with #
multiple line comment can still with {%comment%}
{% endcomment%}

[0kzh]

Since templating engines "inject code," it makes sense to use # like we do in Ruby, for consistency reasons.
I also like @samdoiron 's suggestion as in the following,

{# 
  First line 
  Second line
#}

First line and Second line might be wrongly interpreted as expressions since # conventionally means "ignore everything after this line"

[CalinR]

Definitely prefer to use #. I agree with @ashukasma that it keeps some consistency with Shopify scripts.

I think it make sense to use {# comment #} instead of doing {% # comment %} as you now have to deal with white space control which will cause you to have to do {%- # comment -%}. Instead you could just have {# comment #} remove whitespace by default.

Using {# comment #} also allows syntax highlighting to easily pickup on a whole block of comments.

[bworwa]

OK that makes perfect sense and means that we don't need a new tag at all.

The one question is whitespace management. I'd like for comments to imply {%- -%} whitespace eating. Perhaps we can just introduce (or do we already do that?) a rule that a tag that doesn't output anything itself always eats surrounding whitespace. I imagine this would change resulting HTML, especially with common if and for tags. But... that's probably good?

HTML doesn't care much about whitespace but plain text does. Many use cases of liquid exist around plain text, so if we change the default we definitely need a fallback.

I never understood why whitespaces in control tags were a thing. Having 80+ white lines where a for used to be adds nothing but extra payload to the resulting document. They should all eat whitespaces by default.

I agree on the {% # , the reason why {# is not viable is because it will break inline CSS. E.g: @media(...){#mydiv will break the parser.

[tobi]

Yea, you are right that {# can occur naturally pretty easily. So introducing whitespace clobbering on {% # comment only tags %} feels safe enough.

[samdoiron]

Using {# comment #} also allows syntax highlighting to easily pickup on a whole block of comments.

This is just as easy with {% # this syntax %} since tags can span multiple lines. For example:

{% # 
  Multi-line comments could look like this,
  because tags can span multiple lines.
%}

I never understood why whitespaces in control tags were a thing. Having 80+ white lines where a for used to be adds nothing but extra payload to the resulting document. They should all eat whitespaces by default.

Totally agree, especially in this case. From memory this might be as simple as defining blank? to return true on the node.

[CalinR]

I agree on the {% # , the reason why {# is not viable is because it will break inline CSS. E.g: @media(...){#mydiv will break the parser.

Excellent point. This point alone is good enough to rule out the {# syntax and instead go with {% # to avoid conflicts like this.

#This is just as easy with {% # this syntax %} since tags can span multiple lines. For example:

Good point, this seems like a non issue then.

[isAdrisal]

Definitely agree with @samdoiron on {% # this syntax %}.

Less certain that eating whitespace should be the default behaviour. Two main concerns:

• It's not the standard behaviour of other Liquid tags.
• It can cause unexpected HTML-breaking issues in certain situations. One example:

Liquid:

<video class="foo"
  autoplay
  {% # a comment explaining next line %}
  muted
  src="bar.mp4"
></video>

Invalid output (with auto whitespace removal):

<video class="foo"
  autoplaymuted
  src="bar.mp4"
></video>

Desired output:

<video class="foo"
  autoplay
  muted
  src="bar.mp4"
></video>

Changing {% # to {%- # -%} when typed should be left to IDE shortcuts and user preferences imo.

[CalinR]

I could be wrong, but my understanding is that the way whitespace control currently works is that the output would actually be what you showed in the desired output. It should act the same as a {%- where it would just remove the line that the code block was on and not remove the line break before it.

[isAdrisal]

Perhaps I've misunderstood how this will behave differently to {%- comment -%}{%- endcomment -%}.

Currently, this input:

<video class="foo"
  autoplay
  {%- comment -%}A comment{%- endcomment -%}
  muted
  src="bar.mp4"
></video>

gives this output:

<video class="foo"
  autoplaymuted
  src="bar.mp4"
></video>

[CalinR]

You are 100% correct. Sorry I don't know why I thought it behaved that way. I just tested it out myself too. I agree, that is a bit weird and I guess my misunderstanding shows that people would probably run into that problem often if that was auto assumed.

You're probably right with having it be controlled by the user instead. Other way would be to have the behaviour changed specifically for this instance, but that could possibly produce unexpected results if it was on the same line as other text.

[samdoiron]

are 100% correct. Sorry I don't know why I thought it behaved that way.

I think the assumption that tags don't change data outside of their bounds is the sane one. I can imagine the reasoning for it being to remove extra lines that exist only for comments. But it's definitely weird.

I don't think anyone would want that behaviour of {%- to be the default for comments.

The behaviour I would expect for a comment is that everything from the opening { to the closing } is striped out entirely. The {%- syntax seems to
This may be possible without differing too much from existing behaviour.