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

Use Option<&T> instead of &Option<T> for signal callback parameters #780

Merged
merged 2 commits into from May 22, 2019

Conversation

Projects
None yet
3 participants
@sdroege
Copy link
Member

commented May 22, 2019

See #777

Next step is to do the same for callbacks passed to functions. That apparently does not even use the same machinery to find the correct type mappings between Rust and FFI types.

@GuillaumeGomez Any hint where to look and why you didn't use the trampoline_from_glib function?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented May 22, 2019

Certainly because I didn't know it existed?

@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

Ok, so I might be able to move it over to those functions. Can you tell me where all that type translation magic is that you added here? I would've expected to find a special case somewhere that converts from Option<T> to &Option<T> like there is for signal trampolines, but couldn't find it.

@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

Found it, I guess. No special case, but by-passing the type transformation we do elsewhere ;)

@sdroege sdroege force-pushed the sdroege:option-reference branch from 9b03708 to 9e586c5 May 22, 2019

@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

This should be for all the autogenerated code. Once this is in, things need to be regenerated and manual code needs to be updated.

@EPashkin

This comment has been minimized.

Copy link
Member

commented May 22, 2019

@sdroege Thanks.
Affected gtk, gio, examples

@EPashkin EPashkin merged commit c20ce45 into gtk-rs:master May 22, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

Let's get gtk-rs/gtk#812 merged first? That one is already big enough

@EPashkin

This comment has been minimized.

Copy link
Member

commented May 22, 2019

It's up to you. That PR ready for merge.

@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

Unless you want to regen with this change and then merge, just get it in now and I'll care for it later :)

@EPashkin

This comment has been minimized.

Copy link
Member

commented May 22, 2019

Sorry, I don't want to update 812.

@sdroege

This comment has been minimized.

Copy link
Member Author

commented May 22, 2019

No problem :)

sdroege added a commit to sdroege/gio-rs that referenced this pull request May 23, 2019

@sdroege sdroege referenced this pull request May 23, 2019

Merged

Regenerate #215

sdroege added a commit to sdroege/gtk-rs that referenced this pull request May 23, 2019

@sdroege sdroege referenced this pull request May 23, 2019

Merged

Regenerate #816

sdroege added a commit to sdroege/gtk-rs-examples that referenced this pull request May 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.