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

Support for functions written in go when used as a library #43

Closed
wader opened this issue Sep 24, 2020 · 12 comments · Fixed by #46
Closed

Support for functions written in go when used as a library #43

wader opened this issue Sep 24, 2020 · 12 comments · Fixed by #46
Labels
enhancement New feature or request

Comments

@wader
Copy link
Sponsor Contributor

wader commented Sep 24, 2020

Hello! Would it be possible to add support for own "internal" functions written in go when using gojq as a library?
I had a quick look at the code and it seemed not that far away but maybe i'm missing something? If you think it's a good i would even be happy to try implement it if you give some pointers.

@wader wader changed the title Support for functions written in go when used as library Support for functions written in go when used as a library Sep 24, 2020
@itchyny
Copy link
Owner

itchyny commented Dec 4, 2020

I'm working on this in #46, and you will be able to define your internal function as follows. Any thoughts?

package main

import (
	"fmt"
	"log"

	"github.com/itchyny/gojq"
)

var compilerOptions = []gojq.CompilerOption{
	// gojq.WithFunction({function name}, {arity (argument count)}, {function})
	gojq.WithFunction("f", 0, func(x interface{}, _ []interface{}) interface{} {
		if x, ok := x.(int); ok {
			return x * 2
		}
		return fmt.Errorf("f cannot be applied to: %v", x)
	}),
	gojq.WithFunction("g", 1, func(x interface{}, xs []interface{}) interface{} {
		if x, ok := x.(int); ok {
			if y, ok := xs[0].(int); ok {
				return x + y
			}
		}
		return fmt.Errorf("g cannot be applied to: %v, %v", x, xs)
	}),
}

func main() {
	query, err := gojq.Parse(".[] | f | g(3)")
	if err != nil {
		log.Fatalln(err)
	}
	code, err := gojq.Compile(query, compilerOptions...)
	if err != nil {
		log.Fatalln(err)
	}
	iter := code.Run([]interface{}{1, 2, 3, 4, 5})
	for {
		v, ok := iter.Next()
		if !ok {
			break
		}
		fmt.Printf("%v\n", v)
	}
}

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 4, 2020

Added some comments in the PR. API looks good to me.

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 4, 2020

I do support optional arguments in my hack, but now when i think about it that might be bad and the correct way would be to use multiple WithFunction with different artity and then maybe share go code between them?

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 4, 2020

Rebased and fixed my things on top of your branch, seems to work fine 👍

@itchyny
Copy link
Owner

itchyny commented Dec 4, 2020

I do support optional arguments in my hack, but now when i think about it that might be bad and the correct way would be to use multiple WithFunction with different artity and then maybe share go code between them?

That's exactly what I was concerned. And supporting same function name with different arities introduces a bit hacky code. I'm wondering making the second argument of WithFunction to []int, the list of acceptable arities.

@itchyny
Copy link
Owner

itchyny commented Dec 4, 2020

When I give up supporting adding the functions with the same name, the code would be clean. For example, using gojq.WithFunction("f", []int{0, 1}, F) and gojq.WithFunction("f", []int{2, 3}, G). Supporting all the f/0 to f/3 is difficult.

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 4, 2020

I think in most cases you want min/max arg count range? gojq.WithFunction("f", [2]int{0, 1}, F) or gojq.WithFunction("f", 0, 1, F)? not very pretty.

I guess you don't want to support varargs?

Sorry could you clarify what you mean by "Supporting all the f/0 to f/3 is difficult."? hard to do in the gojq implementation or hard to write custom functions that behave well?

@itchyny
Copy link
Owner

itchyny commented Dec 5, 2020

I think in most cases you want min/max arg count range? gojq.WithFunction("f", [2]int{0, 1}, F) or gojq.WithFunction("f", 0, 1, F)? not very pretty.

I like the idea of giving both min/max arity. It will cover the use case without duplicating the function implementation with the same name.

Sorry could you clarify what you mean by "Supporting all the f/0 to f/3 is difficult."? hard to do in the gojq implementation or hard to write custom functions that behave well?

I was thinking of overwriting the former definition by the latter when the function name conflicts. But I can implement the case by the same way I used to do within the previous implementation accepting single arity. So user don't need to worry about this (see this test for sample).

Thanks for your valuable feedback, I will merge the branch in a few days.

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 5, 2020

Nice! glad I can help. And thank you for gojq, very useful and i've learned a lot while reading the code.

Did you see my comment about passing along customFuncs to def and modules? #46 (comment)

@itchyny
Copy link
Owner

itchyny commented Dec 5, 2020

Did you see my comment about passing along customFuncs to def and modules?

Yes, I was to push my changes after writing tests.

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 5, 2020

@itchyny 👍

@wader
Copy link
Sponsor Contributor Author

wader commented Dec 6, 2020

@itchyny Thanks!

@itchyny itchyny added the enhancement New feature or request label Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants