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

Add helper for waiting for a signal inside a future #32

Closed idanarye closed 3 years ago

idanarye commented 3 years ago

woab::wake_from is probably mostly useful for waiting for a signal. You connect an handler that sends the result and wake the wake_from call. The problem is that the handler remains connected, and even if it doesn't do anything on future invocations (other than trying to send the result and failing) this is still a leak. This may be fine for dialogs, that quickly get tossed away, but if you want to wait on a signal of a more permanent object this can be a problem.

We need an helper that disconnects the signal after waking up.

idanarye commented 3 years ago

The main problem is that in order to disconnect a signal handler, we need both the handler ID and the GLib object. Maybe we can just pass it to the helper:

woab::wake_from_signal(&button, |tx| {
    button.connect_clicked(move |_| {
        let _ = tx.try_send(2);
    })
}).await.unwrap();

I'm half minded to pass it to the closure as argument

woab::wake_from_signal(&button, |button, tx| {
    button.connect_clicked(move |_| {
        let _ = tx.try_send(2);
    })
}).await.unwrap();

But I'm not really sure if that's needed - after all, the closure is not a move one and you don't need a mutable reference to add/remove signals. Passing it to the closure would be useful if you are creating the widget on the spot - but that would usually mean that the widget is also destroyed on the spot, in which case you don't need to worry about disconnecting the signal.

idanarye commented 3 years ago

I was also thinking about connecting the signals directly:

woab::wake_from_signal(&button, "clicked").await;

This has the advantage of being so simple. Problem is - it's only simple for the simple case. GTK signals need to deal with inhibitness:

woab::wake_from_signal(&button, "button-press-event", Some(gtk::Inhibit(true))).await;

And what about parameters? I don't want to always return the parameters struct, because that would require copying even when the user just ignores it. I could pass a function that maps it:

let button = woab::wake_from_signal(
    &button,
    "button-press-event",
    Some(gtk::Inhibit(true)),
    |params| {
        let event: gdk::EventButton = params[1].get::<gdk::Event>().unwrap().unwrap().downcast().unwrap();
        event.get_button()
    },
).await;

But what was supposed to be a simple function is starting to get more and more cumbersome...

One option is to use fluent API:

let button = woab::wake_from_signal(&button, "button-press-event")
    .inhibit(gtk::Inhibit(false))
    .map_params(|params| {
        let event: gdk::EventButton = params[1].get::<gdk::Event>().unwrap().unwrap().downcast().unwrap();
        event.get_button()
    })
    .await;

Maybe even make it usable with the woab::params! macro:

let button = woab::wake_from_signal(&button, "button-press-event")
    .inhibit(gtk::Inhibit(false))
    .map_params(|woab::params!(_, event: gdk::Event)| {
        event.downcast::<gdk::EventButton>().unwrap().get_button()
    })
    .await;

And I could have events (once #22 is done) and actions have their own helper functions:

let button = woab::wake_from_signal(&button, "button-press-event")
    .inhibit(gtk::Inhibit(false))
    .map_event(|event: gdk::EventButton| event.get_button())
    .await;

Now - this is starting to look like a good contender! It keeps the base usage dead simple while still allowing more complex usage!

BTW - I'm not sure we need gtk::Inhibit here. inhibit is already the name of the method, so this should be fine too:

let button = woab::wake_from_signal(&button, "button-press-event")
    .inhibit(false)
    .map_event(|event: gdk::EventButton| event.get_button())
    .await;
idanarye commented 3 years ago

Actually, it would be better to name that second function wait_for_signal - because the user does not need to handle the waking. So maybe I can just implement both?

piegamesde commented 3 years ago

To be honest, this whole "wait for signal" thing is out of my use case scope at the moment.

idanarye commented 3 years ago

I think run_dialog already covers its main usecase, but I still want to write it because doing the right thing (removing the handler after the wake) is hard to do with wake_from.