-
Notifications
You must be signed in to change notification settings - Fork 273
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
Implement server side named parameters #634
base: master
Are you sure you want to change the base?
Conversation
36a3803
to
03b0feb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff!! Looks great, thanks!
Oh I just found out it doesn't work with the meta argument |
Good point, didn't think of that. Can we add a test for this? I guess the solution would be to either:
|
I fixed it, I hope I didn't change too much code.
|
derive/src/to_delegate.rs
Outdated
let param_types: Vec<_> = fn_args.iter().cloned().map(|arg| *arg.ty).collect(); | ||
let arg_names: Vec<_> = fn_args.iter().cloned().map(|arg| *arg.pat).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let param_types: Vec<_> = fn_args.iter().cloned().map(|arg| *arg.ty).collect(); | |
let arg_names: Vec<_> = fn_args.iter().cloned().map(|arg| *arg.pat).collect(); | |
let param_types = fn_args.iter().map(|arg| *arg.ty).collect::<Vec<_>>(); | |
let arg_names = fn_args.iter().map(|arg| *arg.pat).collect::<Vec<_>>(); |
My preference would be to add a generic parameter to collect
, but I'm not super strong on this.
Also .cloned()
doesn't seem required if we dereference/copy the type, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syn::Type
and syn::Pat
don't implement Copy
, so there must be a .clone()
somewhere.
It should be more efficient to clone only syn::Type
or syn::Pat
instead of the whole syn::FnArg
.
No description provided.