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

Add SignalRouter to replace make_signal_handler and connect_signal_handler #12

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

idanarye
Copy link
Owner

I ended up not using using fluent interface to connect multiple signals like I suggested in #11, because actions and widgets have separate connect_local methods that do not originate from the same trait. We probably bridge this with our own trait, but I'm not sure how I feel about doing so. At any rate it's not like connect_signal_handler could do it, so we are not losing functionality (sugar, actually) here.

Once this gets merged, I want to start closing the version. This mostly means bringing the docs and the tests (and the changelog - but this should not be that much work...) up to date, filling the missing parts when necessary. #5 and #7 will have to wait for 0.3 - 0.2 was delayed for long enough.

@idanarye
Copy link
Owner Author

@piegamesde - what do you think about this syntax?

@piegamesde
Copy link
Contributor

because actions and widgets have separate connect_local methods that do not originate from the same trait.

Objection :) I've checked again and for me, both originate from GlibObject.

(I'll have a look at the code some other time.)

@idanarye
Copy link
Owner Author

Could have sworn I looked at it, and somehow I missed it...

I'll add connect.

@idanarye
Copy link
Owner Author

Time's up. I'm merging. We can always change this later.

@idanarye idanarye merged commit 41cb4d9 into master Feb 18, 2021
@piegamesde piegamesde mentioned this pull request Feb 27, 2021
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