sebastienros / fluid

Fluid is an open-source .NET template engine based on the Liquid template language.
MIT License
1.44k stars 178 forks source link

rindex and rindex0 appear to be swapped #686

Closed nairdo closed 2 months ago

nairdo commented 3 months ago

It appears the rindex and rindex0 object property values of the forloop may be reversed (compared to Shopify Liquid):

https://github.com/sebastienros/fluid/blob/aebdaea198efb7c98e788126f80a18e0e84bfd59/Fluid/Ast/ForStatement.cs#L129-L130

compared to ShopifyLiquid: https://github.com/Shopify/liquid/blob/a0411e09277ddba0e12167d56cc2d6a52847ca91/lib/liquid/forloop_drop.rb#L51-L65

    def rindex
      @length - @index
    end
...
    def rindex0
      @length - @index - 1
    end

This issue may also be in the IncludeStatement.cs and RenderStatement.cs.

I know fixing this could possibly cause a bit of grief since some might be expecting the current behavior.

Template

{% assign list = "a, b, c" | split: ", " %}
{%- for item in list %}
 {{ item }} : rindex = {{forloop.rindex}} rindex0 = {{forloop.rindex0}}
{%- endfor %}

Expected output Liquid Sandbox Example

 a: rindex = 3 rindex0 = 2
 b: rindex = 2 rindex0 = 1
 c: rindex = 1 rindex0 = 0

Actual output

 a: rindex = 2 rindex0 = 3
 b: rindex = 1 rindex0 = 2
 c: rindex = 0 rindex0 = 1
sebastienros commented 3 months ago

Looks like a bug, needs to be fixed.

sebastienros commented 2 months ago

@nairdo do you want to submit a PR, it should be easy to fix. You could also add the missing test for that feature.