Better AccelGroup support?

[upsuper]

I'm trying to add some shortcuts to a window without widgets, but found it's quite challenging.

I found that AccelGroup in GTK+ seems to be for this usecase, but it is not very intuitive how to use it with gtk-rs, and I'm suspecting that I would need some unsafe for using it at the moment. Specifically I feel that AccelGroupExt::connect may need a better wrapping method.

It would be greatly appreciated if you can make the binding to AccelGroup easier to use with gtk-rs.

[sdroege]

Why do you think you'll need unsafe code for using it? The way it works currently is that you'd pass a glib::Closure in there, which you can create from fully safe code.

However I agree that this is far from convenient because of the glib::Value handling required :) Do you have any ideas how the API could look like to be more convenient to use?

[upsuper]

Thanks for your reply. I'm glad to hear that this can be done with safe code. Is there any example about how to create a Closure from safe code?

[sdroege]

let c = glib::Closure::new(|args| {
    let arg0 = args[0].get::<WhateverTypeYouExpectHere>().expect("wrong type");
    [...]

    // Return some value if the closure is required to return a value, otherwise `None`
    Some(true.to_value())
});

Note that this will panic if you get the types of the glib::Values wrong.

[upsuper]

It seems this is not so easy. I tried to add a shortcut which changes state of some widget, and the compiler complains about the closure is not Send because GObject's pointer is not Send.

[sdroege]

Use something like fragile::Fragile for this use-case.

We should probably add a new_local() variant to glib::Closure that simply panics if ever called from a different thread then it was created at. That way we could also allow non-Send/non-Sync closures here. I'll try to get that added before the next release.

[upsuper]

Do you have any ideas how the API could look like to be more convenient to use?

There are several things which can be done.

Firstly connect function should be renamed because it conflicts with ObjectExt::connect which means people would need to write qualified name (AccelGroupExt::connect(&accel_group, ...)) to use it.

The second thing would be that it should take a Fn similar to all the other connect_* methods (I think they all take Fn(&Self) + 'static?) that would make it much easier to use I suppose.

I have to admit that I'm not familiar with GTK+ so I may be wrong about how it works. It's just from what I saw when trying to build my UI.

[upsuper]

Also it might be useful to have it accept (u32, ModifierType) (from accelerator_parse) instead of u32 and ModifierType separately, so that people don't need to destruct it just to pass them together to connect again.

[sdroege]

From looking at the docs, it seems like the closure will always have a signature of

gboolean
(*GtkAccelGroupActivate) (GtkAccelGroup *accel_group,
                          GObject *acceleratable,
                          guint keyval,
                          GdkModifierType modifier);

Not sure why it even goes through GClosure at all then. So it would make sense to

1. Rename the function to not conflict with Object::connect()
2. Change from taking a &Closure to the proper signature and doing the translation inside the bindings. Would become Fn(&Self, &Object, u32, ModifierType) -> bool + 'static
3. Unrelated to this, add a Closure::new_local() without the Send bound and panicking if called from the wrong thread

I have to admit that I'm not familiar with GTK+ so I may be wrong about how it works. It's just from what I saw when trying to build my UI.

What are you trying to do exactly?

[sdroege]

I've reported a bug for this against GTK so that they maybe can fix it for GTK4: https://gitlab.gnome.org/GNOME/gtk/issues/2257