-
Notifications
You must be signed in to change notification settings - Fork 82
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
Adds a check for strip_option
when using try_setter
#286
Conversation
One of the tests is failing and I don't know how to solve it. This is my first time working on a proc macro. The test is: #[test]
fn strip_option_try_setter() {
let ty = parse_quote!(Option<Foo>);
let mut setter = default_setter!();
setter.strip_option = true;
setter.try_setter = true;
setter.generic_into = true;
setter.field_type = BuilderFieldType::Optional(&ty);
assert_eq!(
quote!(#setter).to_string(),
quote!(
#[allow(unused_mut)]
pub fn foo<VALUE: ::db::export::core::convert::Into<Foo>>(
&mut self,
value: VALUE,
) -> &mut Self {
let mut new = self;
new.foo = ::db::export::core::option::Option::Some(
::db::export::core::option::Option::Some(value.into()),
);
new
}
pub fn try_foo<VALUE: ::db::export::core::convert::TryInto<Foo>>(
&mut self,
value: VALUE,
) -> ::db::export::core::result::Result<&mut Self, VALUE::Error> {
let converted: Foo = value.try_into()?;
let mut new = self;
new.foo = ::db::export::core::option::Option::Some(
::db::export::core::option::Option::Some(converted),
);
Ok(new)
}
)
.to_string()
);
} The issue is formatting. The assertion fails due to commas:
|
The issue appears to be the commas after the last parameters in the function signatures. Did If so, your best bet is probably to use |
Ah, wasn't aware there was a cfg opt-out for formatting. Nice! |
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.
Directionally looks good; some formatting and git hygiene requests and I think we'll be ready to merge.
I got pulled into a bunch of other changes in other projects due to the release of syn 2, but I'm coming back to this now. |
Looks like the |
@chanced can you rebase this? |
@TedDriggs sorry, missed this earlier. Done. |
Fixes #284.