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

Added glib::source_remove() #145

Merged
merged 2 commits into from Feb 20, 2017

Conversation

Projects
None yet
3 participants
@hfiguiere
Contributor

hfiguiere commented Feb 16, 2017

I could do glib::idle_add() but not remove it, so I wrapped glib::source_remove().

Actually I wonder if that one couldn't be automated, in wish case this PR would totally be wrong.

Thanks.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Feb 16, 2017

Member

Then it might be interesting to wrap the return of glib::idle_add() into an id or something which doesn't provide access to the u32 and make the glib::source_remove() function takes it.

Member

GuillaumeGomez commented Feb 16, 2017

Then it might be interesting to wrap the return of glib::idle_add() into an id or something which doesn't provide access to the u32 and make the glib::source_remove() function takes it.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Feb 18, 2017

Member

@hfiguiere instead make Id field public can you add FromGlib implementation?

Member

EPashkin commented Feb 18, 2017

@hfiguiere instead make Id field public can you add FromGlib implementation?

@hfiguiere

This comment has been minimized.

Show comment
Hide comment
@hfiguiere

hfiguiere Feb 19, 2017

Contributor

Good idea. done.

Contributor

hfiguiere commented Feb 19, 2017

Good idea. done.

Show outdated Hide outdated src/source.rs Outdated
@hfiguiere

This comment has been minimized.

Show comment
Hide comment
@hfiguiere

hfiguiere Feb 20, 2017

Contributor

done.

also this will break gtk-rs. I need to submit a PR for that.

Contributor

hfiguiere commented Feb 20, 2017

done.

also this will break gtk-rs. I need to submit a PR for that.

@hfiguiere

This comment has been minimized.

Show comment
Hide comment
@hfiguiere

hfiguiere Feb 20, 2017

Contributor

Not break. Just consistency.

Contributor

hfiguiere commented Feb 20, 2017

Not break. Just consistency.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Feb 20, 2017

Member

Thanks, @hfiguiere
👍 from me

Member

EPashkin commented Feb 20, 2017

Thanks, @hfiguiere
👍 from me

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Feb 20, 2017

Member

Excellent, thanks for creating the new Id type. It'll help for clarity!

Member

GuillaumeGomez commented Feb 20, 2017

Excellent, thanks for creating the new Id type. It'll help for clarity!

@GuillaumeGomez GuillaumeGomez merged commit da33c35 into gtk-rs:master Feb 20, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment