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

#[prop(default)] uses Default::default(), unless specified. #1776

Closed
wants to merge 2 commits into from
Closed

#[prop(default)] uses Default::default(), unless specified. #1776

wants to merge 2 commits into from

Conversation

k1rowashere
Copy link

#[prop(default)] now acts as #[prop(default = Default::default())].

  • with appropriate test changes

@k1rowashere k1rowashere changed the title #[prop(default)] uses Default::default(), unless specified. #[prop(default)] uses Default::default(), unless specified. Sep 23, 2023
@gbj
Copy link
Collaborator

gbj commented Sep 23, 2023

What's the reasoning here? Isn't this what #[prop(optional)] already does?

@k1rowashere
Copy link
Author

It was just more intuitive to me.
I didn't realize at first that optional worked on non-option types.
Sure, it is a duplication of features but I don't think it has a reason not to work.

@gbj
Copy link
Collaborator

gbj commented Sep 23, 2023

Fair enough. I'd suggest editing it so that #[prop(default)] without an argument is just interpreted as #[prop(optional)], for the sake of reducing maintenance burden/not adding additional code.

@k1rowashere
Copy link
Author

Sadly, the attribute_derive crate doesn't provide attribute aliases, and it will error out if the attribute is not given a value, unless the as_flag method from ConvertParsed is overridden and returns a Some(syn::Expr) instead of None.

It might be possible to merge the implementations of optional and default, but I think it would be more cumbersome/ requires more code.

@tqwewe
Copy link
Contributor

tqwewe commented Oct 25, 2023

+1 for this.
In the past I've done #[prop(default)] and it failed because I also assumed it would use Default::default().

Perhaps in 0.6 #[prop(into)] could even be removed/deprecated in favor of having fewer #[prop(...)] variants - though not sure if people would agree with this

@k1rowashere k1rowashere closed this by deleting the head repository Apr 17, 2024
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.

3 participants