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

Added call_fn, call_native_fn, call_fn_raw and call_native_fn_raw to EvalContext #929

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rawhuul
Copy link

@rawhuul rawhuul commented Nov 2, 2024

Addresses #928

@schungx
Copy link
Collaborator

schungx commented Nov 3, 2024

Great stuff. However, I suggest we defer these functions to the NativeCallContext methods instead of duplicating them? Do you think it can be done? In other words, creating a NativeCallContext from a CallContext...

That way, we keep everything in one place and avoid forgetting to update when things change.

@rawhuul
Copy link
Author

rawhuul commented Nov 4, 2024

Great stuff. However, I suggest we defer these functions to the NativeCallContext methods instead of duplicating them? Do you think it can be done? In other words, creating a NativeCallContext from a CallContext...

Got it! So basically, we need to create a streamlined API that will provide NativeCallContext instead of call_fn methods directly, right? I’ll definitely work on that.

But I have a question: if NativeCallContext already stores the function name, why do we still need to pass the function name as an argument when calling the call_fn* functions?

@schungx
Copy link
Collaborator

schungx commented Nov 5, 2024

Got it! So basically, we need to create a streamlined API that will provide NativeCallContext instead of call_fn methods directly, right? I’ll definitely work on that.

Perhaps duplicate the methods, but inside the implementation create a NativeCallContext and delegate. That might be simpler to the user than having them create a NativeCallContext... although with proper docs it won't be a prob.

But I have a question: if NativeCallContext already stores the function name, why do we still need to pass the function name as an argument when calling the call_fn* functions?

That's because the function name is the name of the current function, while the name passed to call_fn is the function that the current function intends to call...

@rawhuul
Copy link
Author

rawhuul commented Nov 7, 2024

Perhaps duplicate the methods, but inside the implementation create a NativeCallContext and delegate. That might be simpler to the user than having them create a NativeCallContext... although with proper docs it won't be a prob.

Yah, cool. I'll surely look into it.

That's because the function name is the name of the current function, while the name passed to call_fn is the function that the current function intends to call...

Okay, but it seems that the NativeCallContext works with an empty string, it could be Option<&str> then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants