-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
glib: Add get_transformed to Value #1250
base: main
Are you sure you want to change the base?
Conversation
Like transform(), but actually gives you T instead of a new Value, which is probably what you wanted
// rustdoc-stripper-ignore-next | ||
/// Tries to transform the value into the target type | ||
#[doc(alias = "g_value_transform")] | ||
pub fn get_transformed<T: ValueType>(&self) -> Result<T, crate::BoolError> { |
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.
to_transformed()
maybe? It's a conversion, not a getter.
@pbor what do you think?
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.
Is it really more convenient than v.transform().get()
?
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.
Maybe if we want a shortcut we should implement TryInto
?
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.
to_transformed()
maybe? It's a conversion, not a getter.
Yeah I was a bit stumped on naming, get_transformed
as least made sense linguistically but I see your point. I guess the best name really would be transform
…but we already have that.
Is it really more convenient than v.transform().get()?
Well, that doesn't compile, so …yes?
Maybe if we want a shortcut we should implement
TryInto
?
I'd imagine that a TryInto
would want to wrap get()
? — Seems potentially footgun-y to have try_into()
do coercion when get()
doesn't, but idk maybe it would be fine.
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.
Another naming possibility is transform_to
which has a precedent: https://gtk-rs.org/gtk-rs-core/stable/latest/docs/glib/struct.BindingGroupBuilder.html#method.transform_to
though that actually returns Option<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.
the other option is to rename this function to transform
and rename the other transform
function to transform_as_value
or something like that
Any update on this one? |
Like transform(), but actually gives you T instead of a new Value, which is probably what you wanted