Add proc macro for binding methods to action maps

[andy128k]

Closes #240

[sdroege]

This could use some documentation with code example(s) :)

[andy128k]

Yea, sure. I just wanted to share what is done at the moment. Also some tests would be great too.

[bilelmoussaoui]

It looks overall good to me, I just can't see how you set the ActionMap, so an example would help figure out how the API is supposed to work :)

[sdroege]

Seems good to me too apart from the comments above but an example would be nice :)

[bilelmoussaoui]

What I was actually thinking of is to rather uses this with subclasses, and have the gio::actions attribute implement ActionMap/ActionGroup for that subclass automatically and then you can add the actions directly to self as it would be an action map.

[andy128k]

You still may use this macro with subclasses. Just invoke self.register_actions(&self) in a constructor method.

[bilelmoussaoui]

Sure, I guess we can provide another derive macro like the CompositeTemplate for such use case I guess.

[andy128k]

@bilelmoussaoui Frankly, I was under impression that Gtk is moving away from inheritance.

The general direction of our API changes has been to emphasize delegation over subclassing.

many widgets are now final classes

[bilelmoussaoui]

@bilelmoussaoui Frankly, I was under impression that Gtk is moving away from inheritance.

The general direction of our API changes has been to emphasize delegation over subclassing.

many widgets are now final classes

You're mis-understanding what the announcement meant I guess. in gtk3 creating a custom widget was a painful process which was simplified in gtk4, and gtk developers expect you to create custom widgets (a subclass of gtk::Widget) where you compose multiple pre-defined widgets.

[bilelmoussaoui]

Without subclassing you can't use composite templates, properties, signals and all what gtk offers :)

[alatiera]

i Frankly, I was under impression that Gtk is moving away from inheritance.

It's more of "everything now will be inheriting from GtkWidget directly rather than 4 different classes deep"

[sdroege]

Suggested change

	/// fn action1(&self) {
	/// #[action]
	/// fn action1(&self) {

Would probably be better than taking any and every function and turning it into an action

[andy128k]

They are actions. All methods inside this impl item are actions. It is an impl item who is annotated and produces an ActionMap. E.g. it is possible to have more than one ActionMap implemented by a single type.

[sdroege]

I guess that's why I'm asking for a proper example that makes use of most of the features of this :)

[andy128k]

There are two :) One in a doc-test and another in tests.

I also mentioned one in #240. There is a regular impl block with private methods and one more impl block with actions.

[sdroege]

I meant a meaningful example how this would be used in practice as part of the examples or examples4 subdirectory.

Or maybe the docs and docs example just has to be extended to cover these questions too.

[andy128k]

Sure. I will create an example with some realistic actions.

[sdroege]

Does it also handle return values?

[andy128k]

No, it is ignored. Action's activate signal doesn't expect any return value.

[sdroege]

There are stateful actions though. Action::change_state() / Action::get_state()

[andy128k]

This macro doesn't cover/implement stateful actions.

[sdroege]

Please document that then :)

[sdroege]

What if the parameter type in the attribute and the function parameter don't match? Can we infer one from the other to not require both to be present? You can get the GVariant type from a Rust type in any case at runtime.

[andy128k]

It types doesn't match it will be a runtime error (g_critical). I am not sure, it is possible to derive the type without bloating the code (tuples of arbitrary length could be a PITA).

[sdroege]

The traits in glib::variant could be used for this. They already handle exactly these things.

[andy128k]

Oh, I overlooked that StaticVarianType exists. So, it seems I have something to do this evening :)

[andy128k]

This is done. And parameter_type parameter is removed.

[bilelmoussaoui]

Is this really needed? I would rather have everything on the same Impl and decorate all the actions with #[action...]

[andy128k]

I am afraid it is not possible. In this case proc-macro will not be able to generate a single method which gathers all actions into an ActionMap.

[bilelmoussaoui]

You don't need to gather the actions if you assume that self is a glib::IsA<gio::ActionMap> then, you can directly add the actions without requiring an extra method. Of course that would only work for widgets/object that implement ActionMap. Which means gtk::Application & gtk::ApplicationWindow for now. We can then have a proc macro that would automatically implement ActionMap on top of a gobject subclass which would look much cleaner API wise I guess.

[sdroege]

Of course that would only work for widgets/object that implement ActionMap.

Or something that implements Deref<Target=ActionMap> :)

[andy128k]

1. Requiring that Self implements ActionMap makes this macro useless for my personal projects where I do not use inheritance but newtypes to create abstractions.
2. Even if Self: ActionMap is assumed, it doesn't help. Code which generates an action should be invoked from somewhere. Attribute proc-macro attached to impl item allows to append a single method (register_actions in this case) and then this method can be called somewhere. Attribute proc-macro attached to each action handler individually will not be able to do so. It can generate another function and that's it. There is no way to inject invocation of these functions (Unless we assume additional requirement. GtkWidget::created-event would fit here if existed 😆 )

[bilelmoussaoui]

Here's a rough pseudo code idea of what I had in mind

mod imp {

    #[derive(CompositeTemplate, Actions)]
    #[actions(group="widget")]
    pub struct Widget {

    }

    impl ObjectSubclass for Widget {
        ....

        fn type_init(type_: subclass::InitializedType<Self>) {
            type_.add_interface::<gio::ActionMap>();
            type_.add_interface::<gio::ActionGrouo>();
        }
    }

    impl ObjectImpl for Widget {
        fn constructed(&self, obj: &Self::Type) {
            obj.register_actions();
        }
    }

    # both those traits would be automatically implemented when using #[actions(group....)]
    impl ActionMapImpl for Widget {}
    impl ActionGroupImpl for Widget {}
}

glib::wrapper! {
    pub struct Widget(ObjectSubclass<imp::Widget>) @derive gtk::Widget, @implements gio::ActionMap, gio::ActionGroup;
}


impl Widget {

    #[action]
    fn name(&self, action: &gio::Action, target: Option<&glib::Value>) {

    }
}

[andy128k]

    #[action]
    fn name(&self, action: &gio::Action, target: Option<&glib::Value>) {

    }

What a macro should emit here? Notice, that it is inside of an impl block and such block can contain functions or constants only. Nothing else.

[bilelmoussaoui]

Does it have to emit anything actually? In my idea it was mostly used to identify the action functions so that you can register them later

[andy128k]

Of course, it should emit something. At least the function itself 😆 with stripped out attribute (otherwise it won't compile).

[bilelmoussaoui]

Ah yeah, that's what I meant, as in not do anything else other than what the function contains ^^

[andy128k]

Support of stateful actions is added.

[andy128k]

I've done what I planned to do with this macro (at least for the first iteration). 😅

There is one minor issue which is easy to resolve, but I'd like to have a feedback/advice before. Here is an excerpt from Gio documentation.

For stateful actions where the state type is equal to the parameter type, the default is to forward them directly to “change-state”. This should allow almost all users of GSimpleAction to connect only one handler or the other.

With current implementation of the macro this will not work.

#[action(name = "my-action", change-state)]
fn my_action_change_state(&self, state: T) -> Option<T> { ... }

Such declaration of a change-state handler will generate: gio::SimpleAction::new_stateful(None, T::default().to_variant()). So, there is no parameter type in the action's definition. Type of a parameter is taken from a handler of activate signal. So, to make it work, activate handler is needed.

#[action(name = "my-action")]
fn my_action_activate(&self, _state: T, parameter: T) -> Option<T> {
    Some(parameter)
}

#[action(name = "my-action", change-state)]
fn my_action_change_state(&self, state: T) -> Option<T> { ... }

Which is just an ugly boilerplate and cancels the original idea to have only one handler.

So, a possible solution is to add one more annotation to a change-state handler. Something like this:

#[action(name = "my-action", change-state, use-same-parameter-type-as-a-state-type)]
fn my_action_change_state(&self, state: T) -> Option<T> { ... }

But I cannot come up with a good name for it (use-same-parameter-type-as-a-state-type is nothing but a joke 😄).

[sdroege]

@BrainBlasted does this look good to you API-wise now? I'm not doing much with actionmaps, so don't really know :)

[andy128k]

Instead of introducing an opt in flag, I added an opt out one. So, an action parameter type is derived from the state type by default. To opt out a flag no_parameter should be specified.