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

Implement Downcast trait for Option<T> #497

Closed
JMS55 opened this issue Mar 26, 2021 · 12 comments
Closed

Implement Downcast trait for Option<T> #497

JMS55 opened this issue Mar 26, 2021 · 12 comments
Labels
glib good first issue Good for newcomers

Comments

@JMS55
Copy link

JMS55 commented Mar 26, 2021

A lot of the time I do something like this:

widget.get_child().unwrap().downcast::<SomeWidget>().unwrap();

This gets to be really tedious and ugly. I propose gtk-rs should add a function equivalent to the above, something like:

widget.get_child_as::<SomeWidget>();
@sdroege
Copy link
Member

sdroege commented Mar 27, 2021

Sounds useful, yes

@bilelmoussaoui
Copy link
Member

What makes it harder is that get_child is not a WidgetExt method and we will have to add it to a bunch of various types manually. Do you want to send a PR for that? I think having a trait with the provided method implemented for all those types would be an easy way to do it.

@sophie-h
Copy link
Contributor

Is there something special about get_child? Otherwise, I would expect all functions that return a Widget to have a _as variant and I'm not sure if it's worth the effort to hard code that into gir.

@bilelmoussaoui
Copy link
Member

Is there something special about get_child?

Nothing special about it, there are definitely other functions that could benefit from a generic alternative. Do you have any others in mind?

Otherwise, I would expect all functions that return a Widget to have a _as variant and I'm not sure if it's worth the effort to hard code that into gir.

Yeah no, there's no way to detect/generate that automatically from gir

@sophie-h
Copy link
Contributor

Stack::get_visible_child for example and I probably a bunch of functions in libadwaita like hdy_action_row_get_activatable_widget (). I am a bit scared that there will be too much manual stuff at some point and I'm not sure about cost/benefit with this issue.

@ricvelozo
Copy link

Maybe is possible to specialize, something like:

trait OptionCast { /* ... */ } // Similar to Cast trait, but returns the value or panics

impl OptionCast for Option<glib::Object> {
    fn downcast<T: ObjectType>(self) -> T {
        panic!("failed to cast to `{}`", T::static_type())
    }
}

// Now
list_item.get_child().unwrap().downcast::<gtk::Label>().expect("failed to cast to `GtkLabel`");

// After
list_item.get_child().downcast::<gtk::Label>();

@sophie-h
Copy link
Contributor

@bilelmoussaoui I don't think this needs to be on 0.1.0

@JMS55
Copy link
Author

JMS55 commented Apr 11, 2021

Yeah, my idea was to also do it for get_item() and various other functions, but that's a lot of work manually. I'm thinking we would either want to have gir generate a get_x_as() function for any function that looks like get_x() -> Object, but not sure how feasible that is. The much more realistic option, but perhaps slightly worse to use, is what @ricvelozo posted - Add a new function on the Cast trait or somewhere, that at least lets you get rid of two unwraps.

@bilelmoussaoui
Copy link
Member

Maybe is possible to specialize, something like:

trait OptionCast { /* ... */ } // Similar to Cast trait, but returns the value or panics

impl OptionCast for Option<glib::Object> {
    fn downcast<T: ObjectType>(self) -> T {
        panic!("failed to cast to `{}`", T::static_type())
    }
}

// Now
list_item.get_child().unwrap().downcast::<gtk::Label>().expect("failed to cast to `GtkLabel`");

// After
list_item.get_child().downcast::<gtk::Label>();

@sdroege What do you think of something like this? without the panic part. I suppose it's similar to the ListModel::item case where you lose the semantics of the Option in case you're trying to cast to a wrong type.

@sdroege
Copy link
Member

sdroege commented Jul 28, 2021

You could implement Downcast etc on Option<T> and then you'd end up with fn downcast<U>(Option<T>) -> Result<Option<U>, Option<T>>. That would not hide the errors at least while still allowing to directly work on Options.

@bilelmoussaoui
Copy link
Member

I am going to move this to gtk-rs-core as such trait implementation has to be done on glib-rs side

@bilelmoussaoui bilelmoussaoui transferred this issue from gtk-rs/gtk4-rs Jan 20, 2022
@bilelmoussaoui bilelmoussaoui changed the title Add easier get_child() variations Implement Downcast trait for Option<T> Jan 20, 2022
@bilelmoussaoui
Copy link
Member

Fixed in #843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
glib good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants