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

gio: Add type parameter to ListModel and ListStore #609

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jf2048
Copy link
Member

@jf2048 jf2048 commented Mar 14, 2022

Fixes #213

This will break any autogenerated functions using ListModel but we could add a special rule in gir to change that to ListModel<Object>.

Upcasting and downcasting works with a few rules:

  • ListModel<T> is covariant on T
  • ListStore<T> is invariant on T
  • methods of ListStore<T> are contravariant on T

I didn't really change subclassing, that still works the same except you have to do type Interfaces = (gio::ListModel<MyType>,);. There is a little bit more work that needs to be done if someone wants to do a generic object with type Interfaces = (gio::ListModel<T>,);, this PR not far off from that though.

@jf2048
Copy link
Member Author

jf2048 commented Mar 14, 2022

Maybe there should still be a ListStore without a type parameter to support ones with a runtime dynamic type? ListStoreDynamic or something? Or I can change this to TypedListStore<T> and leave ListStore?

@sdroege
Copy link
Member

sdroege commented Mar 14, 2022

Maybe there should still be a ListStore without a type parameter to support ones with a runtime dynamic type?

Wouldn't that be ListStore<glib::Object>?


#[doc(alias = "g_list_model_get_object")]
#[doc(alias = "get_object")]
fn object(&self, position: u32) -> Option<Object>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between get_item / get_object? i don't think we want to have both also it should probably return T maybe?

Copy link
Member Author

@jf2048 jf2048 Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't any real difference in the C API besides the return type. I think fn object can just be removed.

Returning T is inconvenient because then you have to put <T> on ListModelExt and do ListModelExt::<Type>::object(&model, 123) if you want to use the turbofish operator. Putting the parameter on fn item lets you do model.item::<Type>(123). Sadly the return type always has to be specified if we want ListModel<T> to be covariant on T.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why get_item() and get_object() are separate is because get_item() returns a void*, whereas get_object() returns a GObject*. The initial design of GListModel was made generic enough to support any GType payload that could fit into a pointer. Of course, it's completely unbindable ex post, but every now and again somebody thinks they can outsmart every binding in existence.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then perhaps fn object should be kept, and item removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think so, because bindings are supposed to use get_object internally and rename it to get_item; to be consistent with get_n_items

@jf2048
Copy link
Member Author

jf2048 commented Mar 14, 2022

Wouldn't that be ListStore<glib::Object>?

Not unless we want to relax ListStore<T> to also be covariant on T. That can be done but it would be mean that ListStore::insert et al. could fail at runtime again, whereas it can't when it's invariant.

@jf2048
Copy link
Member Author

jf2048 commented Mar 14, 2022

In GTK there are some we know the type of which could be implemented manually with a type parameter:

  • gdk::prelude::DisplayExt::monitors -> gio::ListModel<gdk::Monitor>
  • gtk::prelude::FileChooserExt::files -> gio::ListModel<gio::File>
  • gtk::prelude::FileChooserExt::filters -> gio::ListModel<gtk::FileFilter>
  • gtk::prelude::FileChooserExt::shortcut_folders -> gio::ListModel<gio::File>
  • gtk::Assistant::pages -> gio::ListModel<gtk::AssistantPage>
  • gtk::ColumnView::columns -> gio::ListModel<gtk::ColumnViewColumn>
  • gtk::ConstraintLayout::observe_constraints -> gio::ListModel<gtk::Constraint>
  • gtk::ConstraintLayout::observe_guides -> gio::ListModel<gtk::ConstraintGuide>
  • gtk::Notebook::pages -> gio::ListModel<gtk::NotebookPage>
  • gtk::Window::toplevels -> gio::ListModel<gtk::Widget>

All the rest of the custom models should be safe to take a ListModel<Object>

@jf2048
Copy link
Member Author

jf2048 commented Mar 19, 2022

Unfortunately model.type_().is_a() or value.is_type() still can not type check the parameter with this approach, that should probably be documented somewhere to make it clear

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.

Add type parameter to ListModel and ListStore
4 participants