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

Touch up documentation on Wt::WSignal #196

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

limitedAtonement
Copy link
Contributor

The documentation was missing qualifications concerning under which conditions the signal would NOT be disconnected. That is, for instance if you pass a lambda function object (which doesn't inherit from Wt::Core::observable), Wt::WSignal has no way to unregister or disconnect the signal.

@limitedAtonement
Copy link
Contributor Author

By the way, it looks like there's no way to disconnect from a Wt::WSignal, right? After this code:

other->widget_changed().connect([](){std::cout << "bark\n";});

the only way for that signal handler to get disconnected is if other deletes its widget_changed signal (which will remove all listeners)? I guess that makes sense...that's why you have Wt::Core::observable, right?

@RockinRoel
Copy link
Contributor

connect() always returns connection object, on which you can call disconnect(), so you can always disconnect a slot.

The slot isn't really automatically disconnected when the observable goes away, so it's inaccurate to say so. What really happens is that the slot is wrapped in a function that checks whether the observing_ptr is valid before executing the slot. As it is right now, the slot will not be cleaned up; it simply won't do anything.

@limitedAtonement
Copy link
Contributor Author

Thank you for your pointers! I just noticed this undocumented Wt::Signals::connection when perusing the source. I verified that the return value is mentioned in the docs! (https://www.webtoolkit.eu/wt/doc/reference/html/classWt_1_1Signal.html#aed69704ae147dbb551c3074c8214940c) Thank you for the pointer!

@RockinRoel
Copy link
Contributor

That connection object should probably be documented properly, yeah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants