Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issues with functions? #28

Closed
boriwo opened this issue Sep 17, 2016 · 5 comments
Closed

issues with functions? #28

boriwo opened this issue Sep 17, 2016 · 5 comments
Assignees
Labels

Comments

@boriwo
Copy link

boriwo commented Sep 17, 2016

I wrote this function:

    functions := map[string]govaluate.ExpressionFunction{
        "now": func(args ...interface{}) (interface{}, error) {
            return float64(time.Now().UnixNano() / 1e6), nil
        },
    }

Executing 1+now() works but now()+1 yields "Unable to plan token kind: MODIFIER".

More complex arithmetic expressions including now() trigger other errors.

@Knetic Knetic self-assigned this Sep 17, 2016
@Knetic Knetic added the bug label Sep 17, 2016
@Knetic
Copy link
Owner

Knetic commented Sep 17, 2016

Functions with empty parens were a bit tricky to implement. I had thought I had good enough tests to make sure that it worked, but evidently I missed a spot.

Ought to be fixed as of 17f6df8, seems like I just returned the wrong thing from the codepath which handles empty parens. Sorry for the inconvenience!

@Knetic Knetic closed this as completed Sep 17, 2016
@Knetic Knetic reopened this Sep 17, 2016
@Knetic
Copy link
Owner

Knetic commented Sep 17, 2016

Woops, didn't mean to close. I'll let you verify that it works in your cases, first.

@boriwo
Copy link
Author

boriwo commented Sep 17, 2016

Thank you for the quick fix! I confirmed that now()+1 does indeed work now.

But there are other examples that still look funny:

(now()-1) > 3 for example returns a number instead of a boolean.

@Knetic
Copy link
Owner

Knetic commented Sep 18, 2016

This actually uncovered a bug with the parsing phase and minus sign that I'm surprised I hadn't found before now. This case ought to be fixed as of 89c296b, and for good measure I've added a battery of tests around how functions should work with various operators.

Sorry for the churn on this one! Let me know if your cases work with that fix.

@boriwo
Copy link
Author

boriwo commented Sep 19, 2016

My test cases all work now! Thank you!

@Knetic Knetic closed this as completed Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants