Shopify / liquid-c

Liquid performance extension in C.
MIT License
120 stars 25 forks source link

What's the rational behind joining array elements on display? #177

Open Slashek opened 2 years ago

Slashek commented 2 years ago

https://github.com/Shopify/liquid-c/blob/master/ext/liquid_c/vm.c#L117-L130 seems to be responsible for making the test

The current test (https://github.com/Shopify/liquid-c/blob/master/test/unit/variable_test.rb#L124-L126) is implemented as follows:

 def test_write_array
    output = Liquid::Template.parse("{{ ary }}").render({ "ary" => ["foo", 123, ["nested", "ary"], nil, 0.5] })
    assert_equal("foo123nestedary0.5", output)
  end

However, I can hardly find any reason for such behavior. Instead, I would expect the following assertion to pass:

output = Liquid::Template.parse("{{ ary }}").render({ "ary" => ["foo", 123, ["nested", "ary"], nil, 0.5] })
assert_equal('["foo", 123, ["nested", "ary"], nil, 0.5]', output)

Would you consider changing this behavior, which I believe would be equivalent to just removing a couple of lines of code without having to add any new ones https://github.com/Shopify/liquid-c/blob/master/ext/liquid_c/vm.c#L117-L130 ?