tyler-sommer / stick

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

Not parsing filters in {% for %} blocks #3

Closed bluntelk closed 8 years ago

bluntelk commented 8 years ago

When trying to implement the batch twig filter I get a parse error.

Here is a small program to replicate the problem. It contains the test case and the expected output

package main

import (
    "github.com/tyler-sommer/stick"
    "os"
)

func main() {
    env := stick.NewEnv(nil)

    template := "{% for row in items|batch(3, 'No Item') %}{% for item in row %}{{ item }}.{% endfor %}.{% endfor %}"
    println("Expected: 1.2.3..4.5.6..7.8.No Item..")
    data := map[string]stick.Value{"items":[]int{1, 2, 3, 4, 5, 6, 7, 8}}

    print("Actual: ")
    err := env.Execute(template, os.Stdout, data)
    println()

    if nil != err {
        println(err.Error())
    }

}

The programs output is:

~/golang/src$ go run test-stick.go Expected: 1.2.3..4.5.6..7.8.No Item.. Actual: parse: expected "NUMBER", got "STRING_OPEN" on line 1, column 29

I have verified that this template syntax works with twig

tyler-sommer commented 8 years ago

I just realized I haven't responded to this issue-- sorry about that! This is probably a defect in filter parameter parsing, which is around line 68 in parse/parse_expr.go. If you want to take a look, feel free, I'll try to look into this sometime this week.

bluntelk commented 8 years ago

It's all good. I am not in a rush :) If I get some time to look into it I will have a gander

tyler-sommer commented 8 years ago

I've fixed the original issue (which was incorrect handling of numeric parsing). However, the filter as defined in #2 doesnt work, either.

func FilterBatch(ctx stick.Context, val stick.Value, args ...stick.Value) stick.Value {
    if 2 != len(args) {
        // need 2 arguments
        return args
    }
    sl, ok := val.([]stick.Value) // This never evaluates to ok because its a slice of int or float64, not stick.Value.
    if !ok {
        // not a slice of Values
        return val
    }
    // ...

I am thinking of adding some utility functions such as IsArray() and IsMap() that can then be used to determine how to evaluate. However, I don't think that will be enough. There is a function called iterate which uses runtime reflection to iterate over the elements of whatever is passed-- I think we will need to expose something like this as well to deal with the dynamic nature of values in Twig.

I'll leave this open for now as a place for discussing the extension of the Value api.

bluntelk commented 8 years ago

Thanks for fixing the parse error.

As for the FIlterBatch method... Yeah, that was a little bit naive of me :)

I will look into it and see what I can do

tyler-sommer commented 8 years ago

I'm closing this ticket as I believe the issue is resolved. Please open another ticket if you have any other issues. Thanks!