idanarye / woab

Widgets on Actors Bridge - a GUI microframework for combining GTK with Actix
https://crates.io/crates/woab
MIT License
14 stars 1 forks source link

Macro for streamlining parameter extraction from signals #27

Closed idanarye closed 3 years ago

idanarye commented 3 years ago

26 suggests several macros for signal handling, which can generally be divided into to types:

The latter is more complicated and requires more debate, and I'm not sure it's going to get into 0.3. The former is more local and eventually it could be useful to have both - so I want to discuss it separately and maybe get it can get in sooner.

So I'm leaving #26 for the full-handle macro and opening this new ticket for the param extraction macro.

idanarye commented 3 years ago

@piegamesde's implementations was using a trick where an index variable was incremented after each param extraction. An unrolled loop index, if you will.

This had to be done because woab::Signal does not support iterating over the parameters. But... this can be changed.

The simplest solution would be to have a signal.params() method that returns a &[glib::Value] and let the macro do the downcasting. I'd rather not do that - signal.param(i) gives a detailed error message with the signal name and the index of the parameter, information &[glib::Value] will not have.

So instead, I want to create a new type - SignalParam<'a> (a because it borrows the Signal) - and return an Iterator<Item = SignalParam> + ExactSizeIterator. The macro can then expand to something like:

let mut it = msg.params();
let widget: gtk::WidgetType = it.next().ok_or_else(|| /* proper error */)?.get()?;
let arg1: String = it.next().ok_or_else(|| ...)?.get()?;
let arg2: i32 = it.next().ok_or_else(|| ...)?.get()?;
is it.next().is_some() {
    return Err(/* error because there are too many arguments */);
}

Where get()? would put all the relevant information (gotten from the Signal it was borrowing) in the error.

piegamesde commented 3 years ago

To be honest, this is a lot of boilerplate for little convenience. The index variable got replaced with an it, and accessing it is more tedious than before. I suggest simply exposing a params method in case it is needed, but continue to use the good param(index) API in the macro.

Independently from this, the macro leaks a variable for the iteration into caller scope. Here's my solution around this:

macro_rules! all_args {
    ($signal:expr $(, $var:ident: $type:ty)* $(,)?) => {
        let ($($var, )*) = {
            let signal: woab::Signal<_> = $signal;
            let mut index = 0;
            $(
                let $var: $type = signal.param(index)?;
                index += 1;
            )*
            ($($var, )*)
        };
    };
}
idanarye commented 3 years ago

I think I'll implement it with a proc macro anyway, because I want to support _ for parameters we don't want to handle. WoAB already payed the fixed overhead of having proc macros, and with a proc macro we won't need to use the $index trick. We can also get better compilation errors with a proc macro.

idanarye commented 3 years ago

I also want to try something like this:

let woab::params!(_, foo: Foo, bar: Bar) = msg.params()?;

Which will need some trait work.

piegamesde commented 3 years ago

Does it already handle the #[signal(event)] equivalent?

idanarye commented 3 years ago

No. I'll support events in #22, with a standalone function (msg.event_param()), because events always come as a sole parameter (other than the widget, which you would usually ignore). But I need the gtk-rs team to do a GDK release before I can support that.