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

Various minor fixes #673

Merged
merged 4 commits into from Dec 8, 2018

Conversation

Projects
None yet
2 participants
@sdroege
Copy link
Member

sdroege commented Dec 8, 2018

commit bcfdfe6 (HEAD -> raw-strings)
Author: Sebastian Dröge sebastian@centricular.com
Date: Sat Dec 8 10:58:23 2018 +0200

Also generate display trait for shorthand types only if the global default says so

commit 6178a52
Author: Sebastian Dröge sebastian@centricular.com
Date: Sat Dec 8 10:48:03 2018 +0200

Only insert casts in property getters/setters if we're generating them for a trait

Otherwise compilation fails because type annotations would be needed.

commit 83a8280
Author: Sebastian Dröge sebastian@centricular.com
Date: Sat Dec 8 10:33:38 2018 +0200

Use raw 0-terminated strings for connecting to signals and accessing properties

Going through `s.to_glib_none().0` involves another allocation and copy.

sdroege added some commits Dec 8, 2018

Use raw 0-terminated strings for connecting to signals and accessing …
…properties

Going through `s.to_glib_none().0` involves another allocation and copy.
Only insert casts in property getters/setters if we're generating the…
…m for a trait

Otherwise compilation fails because type annotations would be needed.
@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 8, 2018

Depends on gtk-rs/glib#406

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Dec 8, 2018

@sdroege Thanks, looks good and result build fine locally.
I hope #672 (comment) fixed too.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 8, 2018

I hope #672 (comment) fixed too.

Yes, that's the change to handle in_trait

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Dec 8, 2018

Ok, then lets merge, thanks again

@EPashkin EPashkin merged commit d9c08e1 into gtk-rs:master Dec 8, 2018

1 of 2 checks passed

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

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 8, 2018

https://gitlab.freedesktop.org/slomo/gstreamer-rs/-/jobs/59171

Somewhere we're missing to use glib still. And the IsA<glib::object::Object> bound is not even needed there, only the other one :)

Will look tomorrow if you're not faster

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Dec 8, 2018

Gtk-rs fails with same error on 1.28.0 too.
Next fix works and even don't produce "newer used "use glib" warnings",
but it don't equal generating code in https://github.com/gtk-rs/gir/blob/master/src/codegen/properties.rs#L120

@@ -108,6 +108,7 @@ pub fn analyze(
             if let Some(ref bound, ..) = prop.bound {
                 if bound.bound_type.need_isa() {
                     imports.add("glib::object::IsA", prop.version);
+                    imports.add("glib", prop.version);
                 }
             }

Note: current version of glib/src/gstring.rs fails in 1.28.0 to without "use std;" which is strange for me.
cc @philn

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 8, 2018

Note: current version of glib/src/gstring.rs fails in 1.28.0 to without "use std;" which is strange for me.

You need to use std to be able to do std::bla. ::std::bla is what works without.

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 8, 2018

Thanks for the fix above, I'll take a look at that (and removal of the additional bound) in a bit. Ideally this should only have the bound for IsA and nothing else.

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.