Allow immutable signals #1317
Allow immutable signals #1317
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1317 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 75 75
Lines 12772 12768 -4
=========================================
- Hits 12772 12768 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BenFordTytherington
force-pushed
the
immutable-signals
branch
from
e0d357c to
5dbb156
Compare
BenFordTytherington
force-pushed
the
immutable-signals
branch
from
5dbb156 to
c25611d
Compare
| // // CODECOV_EXCLUDE_START | ||
| // unreachable!("Signals cannot be immutable right now so this cannot be reached") | ||
| // // CODECOV_EXCLUDE_STOP |
There was a problem hiding this comment.
can remove the commented code :-)
There was a problem hiding this comment.
guess we need to ensure there is a unit test that reaches this for coverage though
| "signals must be mutable, use Pin<&mut T> instead of T for the self type", | ||
| )); | ||
| } | ||
| // TODO: add proper checks |
There was a problem hiding this comment.
right, as @LeonMatthesKDAB suggested, we probably only want const signals support inside unsafe extern "C++Qt" blocks not unsafe extern "RustQt", so we need to determine that info somehow here 🤔
| #[qsignal] | ||
| fn const_ready(&self); |
There was a problem hiding this comment.
this would need to move up into the C++Qt block to be on the QTimer
| /// const signal that is emitted when trigger is fired | ||
| #[qsignal] | ||
| #[rust_name = "triggered_const_signal"] | ||
| fn triggeredConstSignal(&self); |
There was a problem hiding this comment.
you should add a call to this here
https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/cpp/external_qobject.cpp#L18
And copy the connection here
https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/rust/src/externcxxqt.rs#L69
And then you can add a signal spy for it and check that they are triggered here so it's tested :-)
https://github.com/KDAB/cxx-qt/blob/main/examples/qml_features/tests/tst_externcxxqt.qml#L41
Fix for #578