search

tyler-sommer / stick

A golang port of the Twig templating engine
MIT License
183 stars 36 forks source link

fix for loop #49

Closed christianWilling closed 3 years ago

christianWilling commented 3 years ago

i fixed multiple problems with this.

first is to use setLocal in the loop so a loop with a loop inside would not override the parent loop variable, which is currently a thing. Than i added Revindex, Revindex0, First and Length according to the twig spec. And than i changed the loop attributes to be lowercase also acording to the twig spec.

https://twig.symfony.com/doc/1.x/tags/for.html

only problem with this is that it will brake compatibility with older versions of this package.

And i could't test the tests they always fail, so maybe test my new tests...

tyler-sommer commented 3 years ago

This is great! The tests look fine, given the fully passing status of CI.

I do worry about the BC break, but bringing it in line with Twig is much more ideal. One solution would be to expose both sets of keys (Titlecase and the corrected lowercase). In the theoretical 2.x version of stick we could then fully remove the Titlecase versions. Does that sound reasonable to you?

christianWilling commented 3 years ago

Yes of course so every one should bei happy.

And over the next werks i will most likely add all the missing filters to get fully ready with the spec. Since I will use this in my company and customers will write there own templates. By the way which version is the correct one 1 2 or 3 ?

tyler-sommer commented 3 years ago

By the way which version is the correct one 1 2 or 3 ?

It would be Twig 1.x that was around when stick was started. However, looking through the changes between 1.x and 2.x it seems like they were mostly internal API changes, not changes to the language itself. If the same holds true for 3.x then I don't see a problem with officially targeting 3.x.

christianWilling commented 3 years ago

According to https://twig.symfony.com/doc/2.x/deprecated.html and https://symfony.com/blog/preparing-your-applications-for-twig-3 there only changes with blocks and class names (which are no problem since wear using no PHP)

christianWilling commented 3 years ago

fixed the backwards compatibility

tyler-sommer commented 3 years ago

Awesome stuff! Thanks again for taking the time to do this, @christianWilling -- it is very much appreciated.