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

Update for non-generic pointer array impls #142

Merged
merged 1 commit into from Jul 18, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Jul 15, 2017

In addition it might make sense to also implement the new traits for the
types that don't use glib_wrapper!()

See gtk-rs/glib#198

Show outdated Hide outdated src/lib.rs
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 15, 2017

Member

Agree that implementing traits without glib will be good but currently IMHO better postpone this until all gtk-rs crates will be updated (if this don't make cairo build fails without glib).

Member

EPashkin commented Jul 15, 2017

Agree that implementing traits without glib will be good but currently IMHO better postpone this until all gtk-rs crates will be updated (if this don't make cairo build fails without glib).

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 15, 2017

Member

Agree that implementing traits without glib will be good but currently IMHO better postpone this until all gtk-rs crates will be updated (if this don't make cairo build fails without glib).

I meant implementing the traits that are not implemented generically anymore, and would be implemented by glib_wrapper!(). But here there are types that don't use glib_wrapper!() and currently implement ToGlibPtr and others manually.

Member

sdroege commented Jul 15, 2017

Agree that implementing traits without glib will be good but currently IMHO better postpone this until all gtk-rs crates will be updated (if this don't make cairo build fails without glib).

I meant implementing the traits that are not implemented generically anymore, and would be implemented by glib_wrapper!(). But here there are types that don't use glib_wrapper!() and currently implement ToGlibPtr and others manually.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 15, 2017

Member

Ah, I missed point, yes it will be needed if it used as array.
Check gtk, gdk show that it not case for cairo and pango.

Member

EPashkin commented Jul 15, 2017

Ah, I missed point, yes it will be needed if it used as array.
Check gtk, gdk show that it not case for cairo and pango.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 15, 2017

Member

I think it's fine nowadays, but will know once I got everything compiling (currently at gdk, then gtk. gdk has some problems)

Member

sdroege commented Jul 15, 2017

I think it's fine nowadays, but will know once I got everything compiling (currently at gdk, then gtk. gdk has some problems)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 18, 2017

Member

And some use need #[cfg(feature = "use_glib")]

warning: unused import: `std::ptr`
 --> src\font\font_options.rs:5:5
  |
5 | use std::ptr;
  |     ^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

warning: unused import: `std::mem`
 --> src\font\font_options.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^

warning: unused import: `std::ptr`
 --> src\font\font_face.rs:5:5
  |
5 | use std::ptr;
  |     ^^^^^^^^

warning: unused import: `std::mem`
 --> src\font\font_face.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^

warning: unused import: `std::mem`
 --> src\font\scaled_font.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^
Member

EPashkin commented Jul 18, 2017

And some use need #[cfg(feature = "use_glib")]

warning: unused import: `std::ptr`
 --> src\font\font_options.rs:5:5
  |
5 | use std::ptr;
  |     ^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

warning: unused import: `std::mem`
 --> src\font\font_options.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^

warning: unused import: `std::ptr`
 --> src\font\font_face.rs:5:5
  |
5 | use std::ptr;
  |     ^^^^^^^^

warning: unused import: `std::mem`
 --> src\font\font_face.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^

warning: unused import: `std::mem`
 --> src\font\scaled_font.rs:6:5
  |
6 | use std::mem;
  |     ^^^^^^^^
Update for non-generic pointer array impls
In addition it might make sense to also implement the new traits for the
types that don't use glib_wrapper!()
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 18, 2017

Member

Done, thanks

Member

sdroege commented Jul 18, 2017

Done, thanks

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 18, 2017

Member

In travis log even more warnings.

Member

EPashkin commented Jul 18, 2017

In travis log even more warnings.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 18, 2017

Member

No warnings here in that configuration: it requires the glib pull request for the warnings to disappear.

Member

sdroege commented Jul 18, 2017

No warnings here in that configuration: it requires the glib pull request for the warnings to disappear.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 18, 2017

Member

Forgot to say that warnings appeared when build without glib

Member

EPashkin commented Jul 18, 2017

Forgot to say that warnings appeared when build without glib

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 18, 2017

Member

I don't get any warnings here when build with --disable-default-features

Member

sdroege commented Jul 18, 2017

I don't get any warnings here when build with --disable-default-features

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 18, 2017

Member

Sorry, forgot that it builds on old glb. All good

Member

EPashkin commented Jul 18, 2017

Sorry, forgot that it builds on old glb. All good

@GuillaumeGomez GuillaumeGomez merged commit 7fc1c52 into gtk-rs:master Jul 18, 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