[valor-plugin] Add support for structs #9
Conversation
examples/hello-plugin/structures.rs
Outdated
| use valor::*; | ||
|
|
||
| #[vlugin] | ||
| #[derive(Default)] |
There was a problem hiding this comment.
This is necessary because RequestHandler::handle_request expects an instance
There was a problem hiding this comment.
Sure, I think it can be polished in #8 luckily it's not needed later to have a that magical feeling of "Add macro and just works"
|
|
||
| Error::new( | ||
| proc_macro2::Span::mixed_site(), | ||
| "Only functions and structures that implement `RequestHandler` are currently supported", |
src/registry.rs
Outdated
| @@ -56,7 +57,7 @@ impl PluginRegistry { | |||
| Box::new(move |mut req: Request| { | |||
| let registry = self.clone(); | |||
| let loader = loader.clone(); | |||
| Box::pin(async move { | |||
| let rslt: HandlerResponse = Box::pin(async move { | |||
There was a problem hiding this comment.
Would you say using as ... is an anti-pattern? matter of preference? :)
There was a problem hiding this comment.
Sometimes as is dangerous, for example, 1000u128 as u8.
All as where removed because trivial_casts was on and I am going to revert these specific changes to remove noise
examples/hello-plugin/functions.rs
Outdated
| use valor::*; | ||
|
|
||
| #[vlugin] | ||
| fn hello_plugin(_req: Request) -> Response { | ||
| "Hello Plugin!".into() | ||
| } | ||
|
|
||
| fn main() {} |
There was a problem hiding this comment.
Since the intended use is as dynlib, making the examples executable might be confusing. It would be cool if after building the examples and running valor-native the user can register them(currently only with the _plugins endpoint) and see how they respond. Perhaps a script can take care of the building, running the api and register them? E.g. in https://github.com/valibre-org/vln-api I created a Makefile to compile the plugins and put them in a directory that is more accessible(that can later be published). Maybe when implementing #6 can get nicer if the plugins are auto-loaded from a directory or passing a file that can be easily deserialized to a collection of Plugins.
There was a problem hiding this comment.
Sorry for the lack of attention. I will revert it back to a library
examples/hello-plugin/structures.rs
Outdated
| use valor::*; | ||
|
|
||
| #[vlugin] | ||
| #[derive(Default)] |
There was a problem hiding this comment.
Sure, I think it can be polished in #8 luckily it's not needed later to have a that magical feeling of "Add macro and just works"
valor-vlugin/src/lib.rs
Outdated
| #[no_mangle] | ||
| pub extern "Rust" fn get_request_handler() -> Box<dyn valor::RequestHandler> { | ||
| Box::new({ | ||
| let fun = |req| { |
There was a problem hiding this comment.
Do you need the wrapping function if instance is already a RequestHandler?
|
Looks good! :) |
Fixes #3
Also adds basic documentation and
Debugimplementations