Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callbacks for invoke_from_event_loop() and Weak::upgrade_in_event_loop() should support return values #5329

Open
npwoods opened this issue Jun 1, 2024 · 3 comments
Labels
a:language-rust Rust API and codegen (mO,mS) breaking change Anything that probably require a semver bump enhancement New feature or request

Comments

@npwoods
Copy link

npwoods commented Jun 1, 2024

Is there a reason that callbacks for invoke_from_event_loop() and Weak::upgrade_in_event_loop() do not support return values?

Currently I have to write code that looks like this:

let cancelled = Arc::new(AtomicBool::new(false));
let cancelled_clone = cancelled.clone();

dialog_weak
	.upgrade_in_event_loop(move |dialog| {
		...
		cancelled_clone.store(dialog.get_cancelled(), Ordering::Relaxed);
	})
	.unwrap();
let cancelled = cancelled.load(Ordering::Relaxed);

It would be great if I could do this:

let cancelled = dialog_weak
	.upgrade_in_event_loop(move |dialog| {
		...
		dialog.get_cancelled()
	})
	.unwrap();

Of course, the return value would have to support Send but that sort of thing goes with the territory.

@ogoffart ogoffart added breaking change Anything that probably require a semver bump enhancement New feature or request a:language-rust Rust API and codegen (mO,mS) labels Jun 4, 2024
@ogoffart
Copy link
Member

ogoffart commented Jun 4, 2024

They could return a Future such as slint::JoinHandle.
This would unfortunately be a breaking change, I think. So If we want to have support of this before Slint 2.0, it would need to be another function name.

@ogoffart
Copy link
Member

ogoffart commented Jun 4, 2024

Note that the code that you wrote with an atomic will not work, because the callback will be run in another thread and can be run after you check the cancelled

As of now, you can use tokio channel.

For example (untested, please let me know if you find mistakes)

let (tx, rx) = tokio::sync::mpsc::channel(1);
dialog_weak.upgrade_in_event_loop(move |dialog| {
   let c = dalog.get_cancelled();
   tx.send(c).unwrap();
}).unwrap();

let cancelled = rx.recv().await.unwrap();

You can use std's channel if you don't want to use async channel.

@npwoods
Copy link
Author

npwoods commented Jun 4, 2024

Yes, I learned (the hard way) that what I proposed wouldn't work - and yes there are plenty of ways to snake a return value out of such a callback, but none of them would be as convenient as a hypothetical invoke_from_event_loop_with_retval() function or the equivalent.

I'll defer to you guys what should happen to this specific GitHub issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:language-rust Rust API and codegen (mO,mS) breaking change Anything that probably require a semver bump enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants