liquid icon indicating copy to clipboard operation
liquid copied to clipboard

Allow `empty` to behave like an empty array (change needed in liquid-c)

Open er1 opened this issue 6 years ago • 6 comments

This allows the empty literal to act as an empty array when being used outside of comparisons. It would make empty array constructions such as {% assign x = '' | split: '' %} more readable as {% assign x = empty %} This should not impact existing uses since empty array will get coerced into empty string if and where it is being used.

Some tests are failing and depend on: https://github.com/Shopify/liquid-c/pull/131

er1 avatar Apr 01 '19 06:04 er1

@er1 This looks like a breaking-change to me. Can you test if the empty array can be filled later on..?

{% assign dropship = empty %}
{% if product.price < store.sale_price %}
  {{ dropship | push: product }}
{% endif %}

ashmaroli avatar Apr 01 '19 11:04 ashmaroli

@ashmaroli It would depend on how that filter works. I am assuming that values are immutable and that the standard filters coerce to arrays when they need arrays before performing operations. This would be in line with how enumerable drops behave.

er1 avatar Apr 05 '19 22:04 er1

@er1 IMO, {% assign x = empty %} doesn't improve readability. It isn't clear if x is an empty string or an empty array. It will take time for existing users to adjust to the fact that empty is a flexible entity now. Would it be possible to introduce [] or empty_array instead..?

ashmaroli avatar Feb 11 '20 10:02 ashmaroli

Would it be possible to introduce [] or empty_array instead..? @ashmaroli I agree that it looks a little unclear at first glance however I don't feel comfortable suggesting new literal names into the language. Existing code might have empty_array set via assigns and introducing a new literal this way will break that code since literals cannot be assigned. Using [] doesn't doesn't feel right unless we flat out introduce array literals into the language as I think it would suggest that something like [2,3,5] or [product1, product2] could be done and that is a much large discussion to have.

er1 avatar Feb 11 '20 13:02 er1

Using [] doesn't doesn't feel right unless we flat out introduce array literals into the language

I see. Thanks for the feedback.

ashmaroli avatar Feb 11 '20 14:02 ashmaroli

failing tests in df49715 depend on Shopify/liquid-c#131

er1 avatar Nov 26 '20 02:11 er1