Accept FnOnce in Handle::update

[crumblingstatue]

This allows closures that capture local variables mutably, allowing more freedom when reacting to state changes in the Tray.

[crumblingstatue]

It's also possible to use FnOnce since the function is only called once, but I don't know if you would want to commit to that API. FnOnce would be the most general thing we could accept, and allow the most freedom at the call site.

[crumblingstatue]

Here is an example of my usage that requires FnMut:

loop {
    let mut should_toggle_window = false; // <- mutably captured
    let mut should_quit = false; // <- mutably captured
    app.tray_handle.update(|tray| {
        if tray.should_toggle_window {
            should_toggle_window = true;
            tray.should_toggle_window = false;
        }
        if tray.should_quit {
            should_quit = true;
            tray.should_quit = false;
        }
    });
    if should_quit {
        break;
    }
    ...

[crumblingstatue]

Alternatively, we could have an inspect method that doesn't update the tray, and just gives temporary access to inspect the state of the Tray. Actually, my example requires mutation of the Tray state.

[iovxw]

Here is an example of my usage that requires FnMut:

loop {
  let mut should_toggle_window = false; // <- mutably captured
  let mut should_quit = false; // <- mutably captured
  app.tray_handle.update(|tray| {
      if tray.should_toggle_window {
          should_toggle_window = true;
          tray.should_toggle_window = false;
      }
      if tray.should_quit {
          should_quit = true;
          tray.should_quit = false;
      }
  });
  if should_quit {
      break;
  }
  ...

Busy loop is an antipattern, please use other synchronization primitives instead

If I understand your usage correctly:

use std::sync::mpsc;

enum Signal {
    ToggleWindow,
    Quit,
}

struct MyTray {
    message_channel: mpsc::Sender<Signal>
}

impl MyTray {
    fn new() -> (Self, mpsc::Reciver) {
        let (tx, rx) = mpsc::channel();
        (Self { message_channel: tx }, rx)
    }
    fn toggle_window(&self) {
        self.message_channel.send(Signal::ToggleWindow)
    }
    fn quit(&self) {
        self.message_channel.send(Signal::Quit)
    )
}

impl ksni::Tray for MyTray {
    fn menu(&self) -> Vec<ksni::MenuItem<Self>> {
        vec![
            StandardItem {
                label: "Quit".into(),
                activate: Box::new(|tray| tray.quit()),
                ..Default::default()
            }
        ]
    }
}

///////////////////////////////////////////////////////////////////////////

let (tray, reciver) = MyTray::new();

// ...

while let signal = reciver.recive() {
    match signal {
       ToggleWindow => // ...
       Quit => break;
    }
}

FnOnce would be the most general thing we could accept, and allow the most freedom at the call site.

You are right, the restriction of Fn is unnecessary here, FnOnce would be better

[crumblingstatue]

Busy loop is an antipattern, please use other synchronization primitives instead

It's not really a busy loop, this is a main loop that runs at 60 fps. But I'll consider switching to another method of communication.

You are right, the restriction of Fn is unnecessary here, FnOnce would be better

If I change the pull request to use FnOnce, are you considering merging it?

[iovxw]

If I change the pull request to use FnOnce, are you considering merging it?

Yes

[crumblingstatue]

Alright, it's taking FnOnce now.

[iovxw]

Thank you👍