-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Partially nested types in properties: newtypes, ThreadGuard #984
base: main
Are you sure you want to change the base?
Conversation
Nesting multiple writable types is still not supported. |
I guess there's still one problem: the setter on the wrapper type accepts the inner type, that is, CC: @YaLTeR |
Yeah, it would be good if the generated public-facing methods had newtype in their signatures. |
#[property(get, set)] | ||
thread_guard_wrapped: Mutex<ThreadGuard<u32>>, | ||
#[property(get, set)] | ||
thread_guard_wrapping: ThreadGuard<Mutex<u32>>, |
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.
Would this also already handle a Mutex<Option<ThreadGuard<Paintable>>>
? Asking for the Option
there. It's a write-only property so I think that potentially works because there's From<T> for Option<T>
but I think it doesn't work in practice because you'd need to call from()
twice?
#crate_ident::PropertySetNested::set_nested( | ||
&self.#field_ident, | ||
move |v| v.#member = #crate_ident::Value::get(value)#expect | ||
move |v| v.#member = ::std::convert::From::from(value) | ||
); | ||
} | ||
), |
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.
Adding a discussion item for this somewhere so we don't forget
I guess there's still one problem: the setter on the wrapper type accepts the inner type, that is, i32, not MyInt(i32).
Would you mind rebasing this one? |
I can rebase, but I don't think this is ready to be merged. There are still some unresolved things, as pointed out by the unresolved comments |
fixes #983
enables support for a correct handling of
ThreadGuard
, therefore supersedes #968.In general, this kinda enables two-level nesting: this makes supporting newtypes trivial (conceptually, they are a nested type).
The change is backwards compatible