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

Finish adding gdk::Atom support to GTK #478

Merged
merged 2 commits into from Mar 28, 2017

Conversation

Projects
None yet
3 participants
@Susurrus
Contributor

Susurrus commented Mar 27, 2017

Still don't know why this type is necessary, but realized I needed to add it after needing it for #472.

Show outdated Hide outdated src/auto/accel_group.rs Outdated
@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 27, 2017

Contributor

I believe this requires additional changes to gdk for the missing FromGlibPtr* impls I think, which should be trivial.

Contributor

Susurrus commented Mar 27, 2017

I believe this requires additional changes to gdk for the missing FromGlibPtr* impls I think, which should be trivial.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 27, 2017

Member

Seems It was bad idea removing impl FromGlibPtrNone<ffi::GdkAtom> for Atom { :(

Member

EPashkin commented Mar 27, 2017

Seems It was bad idea removing impl FromGlibPtrNone<ffi::GdkAtom> for Atom { :(

Show outdated Hide outdated src/auto/text_buffer.rs Outdated
Show outdated Hide outdated src/auto/text_buffer.rs Outdated
Show outdated Hide outdated src/auto/selection_data.rs Outdated
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 27, 2017

Member

Also seems register_serialize_tagset, register_deserialize_tagset, get_target, get-selection and get_data_type has non-optional return

Member

EPashkin commented Mar 27, 2017

Also seems register_serialize_tagset, register_deserialize_tagset, get_target, get-selection and get_data_type has non-optional return

@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 27, 2017

Contributor

I've updated things and now I think we're closer. For those functions erronously returning an Option<_> type, I just ignored them. Those should be addressed by changes to gir or gir-files.

Contributor

Susurrus commented Mar 27, 2017

I've updated things and now I think we're closer. For those functions erronously returning an Option<_> type, I just ignored them. Those should be addressed by changes to gir or gir-files.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

I mean not ignoring non-optional functions but explain it to gir like in https://github.com/gtk-rs/gtk/blob/master/Gir.toml#L325-L326

Member

EPashkin commented Mar 28, 2017

I mean not ignoring non-optional functions but explain it to gir like in https://github.com/gtk-rs/gtk/blob/master/Gir.toml#L325-L326

@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 28, 2017

Contributor

Thanks! I figured there had to be a way to set things like this in the Gir.toml. Here's the latest, but I didn't deal with Clipboard:get_(de)serialize_formats() yet (just set them as ignored) as I'm not positive what they should return. From reading them I think they should just return a Vec<gdk::Atom> as the length doesn't need to be returned as a separate object. If that's the case, then I can implement it manually or should this be something done by gir?

Contributor

Susurrus commented Mar 28, 2017

Thanks! I figured there had to be a way to set things like this in the Gir.toml. Here's the latest, but I didn't deal with Clipboard:get_(de)serialize_formats() yet (just set them as ignored) as I'm not positive what they should return. From reading them I think they should just return a Vec<gdk::Atom> as the length doesn't need to be returned as a separate object. If that's the case, then I can implement it manually or should this be something done by gir?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

Currently Clipboard:get_(de)serialize_formats() need manual implementation. Agree that it need return just vector.

Member

EPashkin commented Mar 28, 2017

Currently Clipboard:get_(de)serialize_formats() need manual implementation. Agree that it need return just vector.

@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 28, 2017

Contributor

Alright, I think that resolves all issues you've brough up @EPashkin. Note that CI won't succeed until gtk-rs/gdk#148 is merged.

Contributor

Susurrus commented Mar 28, 2017

Alright, I think that resolves all issues you've brough up @EPashkin. Note that CI won't succeed until gtk-rs/gdk#148 is merged.

Show outdated Hide outdated src/text_buffer.rs Outdated
@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 28, 2017

Contributor

Sorry, @EPashkin, looks like I was a little too tired last night when I commented, I understand your point about get_(de)serialize_formats() now. This should be ready to roll now!

Contributor

Susurrus commented Mar 28, 2017

Sorry, @EPashkin, looks like I was a little too tired last night when I commented, I understand your point about get_(de)serialize_formats() now. This should be ready to roll now!

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

@Susurrus, Thanks, It seems good.
I check with #478 (comment) just in case in 3 hour.
Then we need wait until @GuillaumeGomez have free time, merged gtk-rs/gdk#148 and restart CI.

Member

EPashkin commented Mar 28, 2017

@Susurrus, Thanks, It seems good.
I check with #478 (comment) just in case in 3 hour.
Then we need wait until @GuillaumeGomez have free time, merged gtk-rs/gdk#148 and restart CI.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 28, 2017

Member

@EPashkin: gtk-rs/gdk#148 needs some modifications so we'll have to wait a bit. ;)

Member

GuillaumeGomez commented Mar 28, 2017

@EPashkin: gtk-rs/gdk#148 needs some modifications so we'll have to wait a bit. ;)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

Strange but get_deserialize_formats still return too big vector that panic on iterate.

Member

EPashkin commented Mar 28, 2017

Strange but get_deserialize_formats still return too big vector that panic on iterate.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 28, 2017

Member

Restarting CI.

Member

GuillaumeGomez commented Mar 28, 2017

Restarting CI.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

Very strange implementation in https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L886-L937
for *_num functions. Num used only for capacity.
Seems this need fix too.

Member

EPashkin commented Mar 28, 2017

Very strange implementation in https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L886-L937
for *_num functions. Num used only for capacity.
Seems this need fix too.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

And others too 😭

Member

EPashkin commented Mar 28, 2017

And others too 😭

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 28, 2017

Member

All CI finished. glib path don't change signatures so can't fail build.

Member

EPashkin commented Mar 28, 2017

All CI finished. glib path don't change signatures so can't fail build.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 28, 2017

Member

Great, then I merge. Thanks @Susurrus!

Member

GuillaumeGomez commented Mar 28, 2017

Great, then I merge. Thanks @Susurrus!

@GuillaumeGomez GuillaumeGomez merged commit 1fd896a into gtk-rs:master Mar 28, 2017

2 checks passed

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

@Susurrus Susurrus deleted the Susurrus:atom_ branch Mar 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment