Reserve future support for comment line before a tag name

handle carriage return in newlines_to_br

.github/workflows/liquid.yml

@@ -6,9 +6,8 @@ jobs:
     strategy:
       matrix:
         entry:
-          - { ruby: 2.5, allowed-failure: false }
-          - { ruby: 2.6, allowed-failure: false }
-          - { ruby: 2.7, allowed-failure: false }
+          - { ruby: 2.5, allowed-failure: false } # minimum supported
+          - { ruby: 3.0, allowed-failure: false } # latest
           - { ruby: ruby-head, allowed-failure: true }
     name: test (${{ matrix.entry.ruby }})
     steps:

Gemfile

@@ -22,6 +22,6 @@ group :test do
   gem 'rubocop-performance', require: false
 
   platform :mri, :truffleruby do
-    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'master'
+    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'inline-comment'
   end
 end

History.md

@@ -1,5 +1,10 @@
 # Liquid Change Log
 
+## Unreleased
+
+### Features
+* Allow `#` to be used as an inline comment tag (#1401) [Dylan Thacker-Smith]
+
 ## 5.0.0 / 2021-01-06
 
 ### Features

lib/liquid/block_body.rb

@@ -4,8 +4,8 @@ require 'English'
 
 module Liquid
   class BlockBody
-    LiquidTagToken      = /\A\s*(\w+)\s*(.*?)\z/o
-    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(\w+)(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
+    LiquidTagToken      = /\A\s*(\w+|#)\s*(.*?)\z/o
+    FullToken           = /\A#{TagStart}#{WhitespaceControl}?(\s*)(\w+|#)(\s*)(.*?)#{WhitespaceControl}?#{TagEnd}\z/om
     ContentOfVariable   = /\A#{VariableStart}#{WhitespaceControl}?(.*?)#{WhitespaceControl}?#{VariableEnd}\z/om
     WhitespaceOrNothing = /\A\s*\z/
     TAGSTART            = "{%"
@@ -99,7 +99,9 @@ module Liquid
     end
 
     private def parse_liquid_tag(markup, parse_context)
-      liquid_tag_tokenizer = Tokenizer.new(markup, line_number: parse_context.line_number, for_liquid_tag: true)
+      liquid_tag_tokenizer = parse_context.new_tokenizer(
+        markup, start_line_number: parse_context.line_number, for_liquid_tag: true
+      )
       parse_for_liquid_tag(liquid_tag_tokenizer, parse_context) do |end_tag_name, _end_tag_markup|
         if end_tag_name
           BlockBody.unknown_tag_in_liquid_tag(end_tag_name, parse_context)

lib/liquid/locales/en.yml

@@ -22,6 +22,7 @@
       tag_never_closed: "'%{block_name}' tag was never closed"
       table_row: "Syntax Error in 'table_row loop' - Valid syntax: table_row [item] in [collection] cols=3"
       render: "Syntax error in tag 'render' - Template name must be a quoted string"
+      inline_comment_invalid: "Syntax error in tag '#' - Each line of comments must be prefixed by the '#' character"
     argument:
       include: "Argument error in tag 'include' - Illegal template name"
     disabled:

lib/liquid/parse_context.rb

@@ -23,6 +23,10 @@ module Liquid
       Liquid::BlockBody.new
     end
 
+    def new_tokenizer(markup, start_line_number: nil, for_liquid_tag: false)
+      Tokenizer.new(markup, line_number: start_line_number, for_liquid_tag: for_liquid_tag)
+    end
+
     def parse_expression(markup)
       Expression.parse(markup)
     end

lib/liquid/standardfilters.rb

@@ -89,13 +89,15 @@ module Liquid
 
     def truncatewords(input, words = 15, truncate_string = "...")
       return if input.nil?
-      wordlist = input.to_s.split
-      words    = Utils.to_integer(words)
+      input = input.to_s
+      words = Utils.to_integer(words)
+      words = 1 if words <= 0
 
-      l = words - 1
-      l = 0 if l < 0
+      wordlist = input.split(" ", words + 1)
+      return input if wordlist.length <= words
 
-      wordlist.length > l ? wordlist[0..l].join(" ").concat(truncate_string.to_s) : input
+      wordlist.pop
+      wordlist.join(" ").concat(truncate_string.to_s)
     end
 
     # Split input string into an array of substrings separated by given pattern.
@@ -295,7 +297,7 @@ module Liquid
 
     # Add <br /> tags in front of all newlines in input string
     def newline_to_br(input)
-      input.to_s.gsub(/\n/, "<br />\n")
+      input.to_s.gsub(/\r?\n/, "<br />\n")
     end
 
     # Reformat a date using Ruby's core Time#strftime( string ) -> string

lib/liquid/tags/inline_comment.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Liquid
+  class InlineComment < Tag
+    def initialize(tag_name, markup, options)
+      super
+      # Semantically, a comment should only ignore everything after it on the line.
+      # Currently, this implementation doesn't support mixing a comment with another tag
+      # but we need to reserve future support for this.
+      if markup.match?(/\n\s*[^#]/)
+        raise SyntaxError, options[:locale].t("errors.syntax.inline_comment_invalid")
+      end
+    end
+
+    def render_to_output_buffer(_context, output)
+      output
+    end
+
+    def blank?
+      true
+    end
+  end
+
+  Template.register_tag('#', InlineComment)
+end

lib/liquid/template.rb

@@ -107,7 +107,8 @@ module Liquid
     # Returns self for easy chaining
     def parse(source, options = {})
       parse_context = configure_options(options)
-      @root         = Document.parse(tokenize(source), parse_context)
+      tokenizer     = parse_context.new_tokenizer(source, start_line_number: @line_numbers && 1)
+      @root         = Document.parse(tokenizer, parse_context)
       self
     end
 
@@ -223,10 +224,6 @@ module Liquid
       parse_context
     end
 
-    def tokenize(source)
-      Tokenizer.new(source, @line_numbers)
-    end
-
     def apply_options_to_context(context, options)
       context.add_filters(options[:filters]) if options[:filters]
       context.global_filter      = options[:global_filter] if options[:global_filter]

performance/theme_runner.rb

@@ -73,10 +73,14 @@ class ThemeRunner
 
   private
 
+  def render_layout(template, layout, assigns)
+    assigns['content_for_layout'] = template.render!(assigns)
+    layout&.render!(assigns)
+  end
+
   def compile_and_render(template, layout, assigns, page_template, template_file)
-    compiled_test                 = compile_test(template, layout, assigns, page_template, template_file)
-    assigns['content_for_layout'] = compiled_test[:tmpl].render!(assigns)
-    compiled_test[:layout].render!(assigns) if layout
+    compiled_test = compile_test(template, layout, assigns, page_template, template_file)
+    render_layout(compiled_test[:tmpl], compiled_test[:layout], compiled_test[:assigns])
   end
 
   def compile_all_tests

test/integration/context_test.rb

@@ -461,6 +461,7 @@ class ContextTest < Minitest::Test
   end
 
   def test_interrupt_avoids_object_allocations
+    @context.interrupt? # ruby 3.0.0 allocates on the first call
     assert_no_object_allocations do
       @context.interrupt?
     end

test/integration/standard_filter_test.rb

@@ -175,6 +175,9 @@ class StandardFiltersTest < Minitest::Test
     )
     assert_equal("测试测试测试测试", @filters.truncatewords('测试测试测试测试', 5))
     assert_equal('one two1', @filters.truncatewords("one two three", 2, 1))
+    assert_equal('one two three...', @filters.truncatewords("one  two\tthree\nfour", 3))
+    assert_equal('one two...', @filters.truncatewords("one two three four", 2))
+    assert_equal('one...', @filters.truncatewords("one two three four", 0))
   end
 
   def test_strip_html
@@ -539,6 +542,7 @@ class StandardFiltersTest < Minitest::Test
 
   def test_newlines_to_br
     assert_template_result("a<br />\nb<br />\nc", "{{ source | newline_to_br }}", 'source' => "a\nb\nc")
+    assert_template_result("a<br />\nb<br />\nc", "{{ source | newline_to_br }}", 'source' => "a\r\nb\nc")
   end
 
   def test_plus

test/integration/tags/inline_comment_test.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class InlineCommentTest < Minitest::Test
+  include Liquid
+
+  def test_tag_in_different_styles
+    assert_template_result('', '{% # This text gets ignored %}')
+    assert_template_result('', '{%# This text gets ignored #%}')
+    assert_template_result('', '{%# This text gets ignored %}')
+    assert_template_result('', '{%#- This text gets ignored -#%}')
+  end
+
+  def test_test_syntax_error
+    assert_template_result('fail', '{% #This doesnt work %}')
+
+    assert false 
+  rescue 
+    # ok good
+  end
+
+  def test_tag_ws_stripping
+    assert_template_result('', '   {%#- This text gets ignored -#%}    ')
+  end
+
+  def test_comment_inline_tag
+    assert_template_result('ok', '{% echo "ok" # output something from a tag %}')
+  end
+
+  def test_comment_line_before_tag
+    assert_template_result('ok', '{% # this sort of comment also
+      echo "ok" %}')
+  end
+
+  def test_comment_inline_variable
+    assert_template_result('ok', '{{ "ok" # output something from a variable }}')
+    assert_template_result('ok', '{{ "OK" | downcase # output something from a variable }}')
+  end
+
+  def test_inside_liquid_tag
+    source = <<~LIQUID
+      {%- liquid
+        echo "before("
+        # This text gets ignored
+        echo ")after"
+      -%}
+    LIQUID
+    assert_template_result('before()after', source)
+  end
+
+  def test_multiline
+    assert_template_result('', '{% # this sort of comment also
+      # will just work, because it parses
+      # as a single call to the "#" tag %}')   
+
+  end
+
+end

test/unit/tokenizer_unit_test.rb

@@ -32,21 +32,26 @@ class TokenizerTest < Minitest::Test
 
   private
 
+  def new_tokenizer(source, parse_context: Liquid::ParseContext.new, start_line_number: nil)
+    parse_context.new_tokenizer(source, start_line_number: start_line_number)
+  end
+
   def tokenize(source)
-    tokenizer = Liquid::Tokenizer.new(source)
+    tokenizer = new_tokenizer(source)
     tokens    = []
-    while (t = tokenizer.shift)
+    # shift is private in Liquid::C::Tokenizer, since it is only for unit testing
+    while (t = tokenizer.send(:shift))
       tokens << t
     end
     tokens
   end
 
   def tokenize_line_numbers(source)
-    tokenizer    = Liquid::Tokenizer.new(source, true)
+    tokenizer    = new_tokenizer(source, start_line_number: 1)
     line_numbers = []
     loop do
       line_number = tokenizer.line_number
-      if tokenizer.shift
+      if tokenizer.send(:shift)
         line_numbers << line_number
       else
         break