Add composite template callback macro

[jf2048]

Fixes #24, also fixes part 3 of #77. Depends on gtk-rs/gtk-rs-core#338.

While getting to this work, I noticed that calling Builder::current_object while inside create_closure caused GTK to crash because from_glib_none has consumed the floating reference. I changed current_object to a manual implementation that calls g_object_ref via clone, and that fixes it. Long-term it seems that BuilderScopeImpl should probably be an unsafe trait since all its methods are able to get access to the builder while the object is being constructed.

[YaLTeR]

This is very cool!

I have a "UX" question: is the , callbacks part necessary on the #[template]? Maybe it can figure out what to do on its own?

[jf2048]

I have a "UX" question: is the , callbacks part necessary on the #[template]?

Not quite, but the only other option I could get to work was to make the template_callbacks macro mandatory for all uses of derive(CompositeTemplate). Even for a widget with no template callbacks you would still need to put that on an empty impl. At least with the callbacks attribute it doesn't complicate the case where a template widget has no callbacks.

This is because the template_callbacks macro needs to generate a trait that can then be used to provide the BuilderScope. If that trait is not present then the class_init will fail to compile if it tries to build a scope. I tried to get it to work by having it figure it out on its own but I couldn't do it without the unstable specialization feature. Taking suggestions on how this could be done otherwise...

[A6GibKm]

The callback argument looks out of place, if it comes to it, doing a macro #[template_with_callbacks] sounds better imo.

is it not possible to do something similar to what the properties method (or overriding vfuncs) do where the user can simply not specify it? Or similar to how templates work where the user has to override an impl in ObjectSubclass

        fn class_init(klass: &mut Self::Class) {
            Self::bind_template(klass);
        }

?

[jf2048]

is it not possible to do something similar to what the properties method (or overriding vfuncs) do where the user can simply not specify it?

This one only works because subclasses have to implement ObjectImpl anyway, so it's no problem to make a default implementation that does nothing.

Or similar to how templates work where the user has to override an impl in ObjectSubclass

This could actually be done by making it Self::bind_template_callbacks(klass); or something like that. But I don't know if that is better than using a macro or an attribute.

[A6GibKm]

But I don't know if that is better than using a macro or an attribute.

In my opinion it works better, it also mirrors the C api better, Anyways, in the future it can be improved to be automatic.

[ids1024]

I tried to get it to work by having it figure it out on its own but I couldn't do it without the unstable specialization feature. Taking suggestions on how this could be done otherwise...

There's a lot of black magic I'd like to try using to improve sub-classing, but would require specialization other features that aren't stable/implemented and won't be soon. Unfortunately I don't think there's currently a way to avoid something like this.

Personally, I would find it more natural to define callbacks on the wrapper type. I might want to make a function part of the type's public API and also use it as a callback; which I think isn't too uncommon. With the current implementation I'd have to define the function for both types, with one wrapping the other.

(It's generally annoying having to deal with a separate private inner type and a wrapper type when sub-classing, but I don't know how that could be avoided.)

[bilelmoussaoui]

I suppose this could use a rebase so the tests are run after merging the requires ObjectExt::watch_closure? :)

[jf2048]

I've spent some time thinking about this and I'd like to rework it a little bit. My current proposal would be to drop , callbacks and instead have three separate macros that would all generate a CompositeTemplateCallbacks trait. These could then be bound in the class_init using the same trait method.

The three macros are distinguished by their handling of the self parameter on template callbacks:

• gtk::template_callbacks_public - For use on the public widget type, self is unwrapped as Self.
• gtk::template_callbacks_private - For use on the ObjectSubclass impl, self is unwrapped as <Self as ObjectSubclass>::Type.
• gtk::template_callbacks_library - For utility functions, using self is an error and the first Value is ignored entirely when calling the function.

Here's a sample of how this could work, note the class_init function:

#[gtk::template_callbacks_public]
impl MyWidget {
    #[template_callback]
    fn button_clicked(&self, button: gtk::Button) {
        // ...
    }
}

struct CallbackLibrary {}

#[gtk::template_callbacks_library]
impl CallbackLibrary {
    #[template_callback]
    fn concat(a: &str, b: &str) -> String {
        format!("{} {}", a, b)
    }
}

mod imp {
    #[derive(Debug, Default, CompositeTemplate)]
    #[template(file = "mywidget.ui")]
    pub struct MyWidget {
        // ...
    }

    #[gtk::template_callbacks_private]
    impl MyWidget {
        #[template_callback]
        fn private_button_clicked(&self, button: gtk::Button) {
            // ...
        }
    }

    #[glib::object_subclass]
    impl ObjectSubclass for MyWidget {
        const NAME: &'static str = "MyWidget";
        type Type = super::MyWidget;
        type ParentType = gtk::Widget;

        fn class_init(klass: &mut Self::Class) {
            Self::bind_template(klass);
            Self::bind_template_callbacks(klass);
            Self::Type::bind_template_callbacks(klass);
            CallbackLibrary::bind_template_callbacks(klass);
        }

        fn instance_init(obj: &glib::subclass::InitializingObject<Self>) {
            obj.init_template();
        }
    }
}

These could also be combined into one macro using attributes if anyone thinks that would work better:

// one of these three is required
#[gtk::template_callbacks(public)]
#[gtk::template_callbacks(private)]
#[gtk::template_callbacks(library)]

It seems that the public and private distinction could also be removed if either ValueType was implemented on ObjectSubclass types, or if specialization was stable and we could use another trait that has separate impls for ValueType and ObjectSubclass.

[A6GibKm]

             Self::bind_template_callbacks(klass);
             Self::Type::bind_template_callbacks(klass);

Do we need both?

[sdroege]

Before merging this we should also figure out gtk-rs/gtk-rs-core#366 .

[jf2048]

Do we need both?

Yes if we want to have callbacks on multiple different structs, with the current implementation as a trait. Possibly that could also be simplified with specialization.

[bilelmoussaoui]

The three macros are distinguished by their handling of the self parameter on template callbacks:

gtk::template_callbacks_public - For use on the public widget type, self is unwrapped as Self.
gtk::template_callbacks_private - For use on the ObjectSubclass impl, self is unwrapped as <Self as ObjectSubclass>::Type.
gtk::template_callbacks_library - For utility functions, using self is an error and the first Value is ignored entirely when calling the function.

As you suggested, I would make those the same macro and have an attribute to define whether you want it to be public/private/library that defaults to public which is the most used use case. I wonder if the last use case can be just an attribute on the the functions themselves instead of the whole trait?

[jf2048]

I wonder if the last use case can be just an attribute on the the functions themselves instead of the whole trait?

Sure, but only for library. The other ones only make sense on the impl since the interpretation of self is always the same. And maybe that name is not so good there, taking suggestions on what to call it...

[bilelmoussaoui]

I wonder if the last use case can be just an attribute on the the functions themselves instead of the whole trait?

Sure, but only for library. The other ones only make sense on the impl since the interpretation of self is always the same. And maybe that name is not so good there, taking suggestions on what to call it...

Yes, that would be useful for things like Expressions

[jf2048]

This still needs docs, but has been updated with a new API:

• #[gtk::template_callbacks] - Unwraps self as Self.
• #[gtk::template_callbacks(private)] - Unwraps self as <Self as ObjectSubclass>::Type.
• #[gtk::template_callbacks(functions)] - By default, makes all callbacks use the function attribute. (see below)

The following are supported as arguments to #[template_callback]:

• name = "func" - Renames the function in the template.
• function - Ignores the first value when calling the callback, therefore doesn't allow self.
• method - Only useful with functions, reverts the effects of it so the callback gets the first value and can take self again. Mainly useful for callbacks that are invoked with swapped="true".

The attribute #[rest] can be placed on the last argument of a template callback. This attribute must be used on an argument of type &[glib::Value] and will pass in the remaining arguments. The first and last values will be omitted from the slice if this callback is a function.

The composite_template example has been updated to make use of each of these.

[BrainBlasted]

In C it is extremely common to cast the last arg (user_data) to Self.

static void
some_action (GSimpleAction *action,
             GVariant      *parameter,
             gpointer       user_data)
{
  MyObj *self = MY_OBJ (user_data);
  
  // ...
}

I've also rarely seen people use the type outright instead of the gpointer - though this is more common with swapped=yes:

static void
some_action (GSimpleAction *action,
             GVariant      *parameter,
             MyObj         *self)
{
 // ...
}

Here is an example.

swapped or not doesn't change the existence of self - it's still gpointer user_data, but convention has it so that people just use self like they would on a member function when connecting signals as swapped.

[bilelmoussaoui]

• #[gtk::template_callbacks(value)] - Goes on the wrapper type. Technically this can be placed on any type that implements FromValue.
• #[gtk::template_callbacks(subclass)] - Goes on the ObjectSubclass type.
• #[gtk::template_callbacks] - Works if the type of the first argument is specified manually, but if you try to use &self then it throws an error asking for either the value or subclass option to be added to the impl.

I don't know, the three variants seems a bit hard to figure out just from reading the "name". In most of the code I wrote using gtk-rs, I think the most common case is the second one, so why not make that one the default ?

Thinking about this and after giving it a bit of try, i think we should only keep two variants:

• The default: [gtk::template_callbacks] goes either on the public/private type (to be decided later)
• #[gtk::template_callbacks(utilities)] meant for shared functions to be used with expressions mainly

[sdroege]

The default: [gtk::template_callbacks] goes either on the public/private type (to be decided later)

Well we need to decide that before merging this, or not?

#[gtk::template_callbacks(utilities)] meant for shared functions to be used with expressions mainly

This would be for plain/associated functions that don't take any &self?

[bilelmoussaoui]

The default: [gtk::template_callbacks] goes either on the public/private type (to be decided later)

Well we need to decide that before merging this, or not?

Yes, let's figure out a way to vote for this ?

#[gtk::template_callbacks(utilities)] meant for shared functions to be used with expressions mainly

This would be for plain/associated functions that don't take any &self?

Yep

[sdroege]

Yes, let's figure out a way to vote for this ?

There are lots of websites to set up votes, or we can just do it on Matrix.

But as people apparently want to do both variants it seems a bit restrictive to disallow the other completely. We should probably select a default but keep the other possible.

[A6GibKm]

They are equivalent, it just happens that one will always be a tiny bit more comfortable to use depending on the usecase. I would also prefer to have just one for the sake of simplicity.

[jf2048]

My preference would be just to pass self as self (if present) always and then pass the rest of the arguments like C.

This can't be done with the way the GTK4 API is now, the callbacks are declared during class initialization before any instances exist, and are then invoked from C as part of the GtkBuilder where the only source of the self reference is from the closure itself. It's mentioned in the documentation that you usually need to place swapped="true" if you want to use a self parameter.

What could be done is a dynamic typecheck against the first and last arguments to see if either of them fulfill self, but I don't know if that's going to work well. If the current object is a widget of the same type, then I don't think there is a logical way to prioritize one over the other to decide which one is really self. Also you'll still get panics if you use object="..." because then there is no way to resolve self at all.

[Cogitri]

I've just ported Health to use composite template callbacks, that sure made my code a lot nicer! I exclusively used #[gtk::template_callbacks(value)], there are only a handful of functions which don't need the wrapped type for me. A few things that I've noticed (a few of them have already been talked about on Matrix, I just figured it'd be good to document them here):

• Signals like GtkSpinButtton's ::input can't be connect via the current macros currently since it uses an out pointer for returning a value.
• gtk::Inhibit currently doesn't implement ToValue, so one has to use bool instead.
• I usually just do swapped="true" on everything and at least for me it wasn't a problem to order parameters that way.
• Apparently the type of the parameter has to match exactly - specifying a superclass doesn't work: [...] panicked at 'Wrong type for argument 1 in template callback handle_distance_spin_button_changed: WrongValueType(ValueTypeMismatchError { actual: HealthUnitSpinButton, requested: GtkSpinButton }). Same for gtk::ResponseType which instead requires you to use i32 and then manually do gtk::ResponseType::from_glib() in the function.

[jf2048]

Thanks for the testing and feedback! Let's try to improve some of it:

• Signals like GtkSpinButtton's ::input can't be connect via the current macros currently since it uses an out pointer for returning a value.

This can't be done safely, it can be done with unsafe, but I'm going to update the docs to clarify some of the behavior here and to match the docs for glib::closure!. I'll also mention that Value can be used as an argument type if custom unboxing is needed, e.g. for pointer types.

• gtk::Inhibit currently doesn't implement ToValue, so one has to use bool instead.

Good catch, here's another PR for that: gtk-rs/gtk-rs-core#430

• Apparently the type of the parameter has to match exactly - specifying a superclass doesn't work

This should actually work, I think it's not working because UnitSpinButton is not a SpinButton, it's an AdwBin? I changed one of the unit tests to use Widget and then downcast it, and that seems to work.

Same for gtk::ResponseType which instead requires you to use i32 and then manually do gtk::ResponseType::from_glib() in the function.

Unfortunately the type in the C signal for response is a G_TYPE_INT so there's not much we can do here. For signals that use the right enum type, it should work as expected.

[bilelmoussaoui]

Overall I'm happy with the current situation 🙂

Would be nice to figure out a way to define what could be the default type of the callbacks between subclass/value

[Cogitri]

This should actually work, I think it's not working because UnitSpinButton is not a SpinButton, it's an AdwBin? I changed one of the unit tests to use Widget and then downcast it, and that seems to work.

Erm right, was tricked by my own bad naming :D

Thanks for the response :)

[jf2048]

I made a PR to add a trait that removes the need for (value) and (subclass): gtk-rs/gtk-rs-core#435

Tentative changes to this patch are here: f2a7b65

[bilelmoussaoui]

Thanks @jf2048 for your work & everyone for their reviews! If there's something else you would like to see improved/fixed please open a new issue / merge request.

[liferooter]

I really can't wait for release with this PR merged. It's so hard to live without this feature after GTK with Python or Vala