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

Fix PixbufFormat, regen Pixbuf #57

Merged
merged 8 commits into from Dec 30, 2017

Conversation

Projects
None yet
3 participants
@EPashkin
Member

EPashkin commented Dec 17, 2017

Closes #55
Closes #56
Closes #23

Build will fails until gtk-rs/glib#278

cc @GuillaumeGomez, @sdroege

@EPashkin EPashkin changed the title from WIP: Fix PixbufFormat, regen Pixbuf to Fix PixbufFormat, regen Pixbuf Dec 17, 2017

@EPashkin

This comment has been minimized.

Member

EPashkin commented Dec 17, 2017

Ok, PR is finished,
Not all async functions generated by unknown reason, but we can finish it later.

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Dec 17, 2017

I agree. Once CI is good, it's good for me as well.

@EPashkin

This comment has been minimized.

Member

EPashkin commented Dec 17, 2017

Forgot to check async function, at minimum new_from_stream_async need be manual.

@sdroege

This comment has been minimized.

Member

sdroege commented Dec 17, 2017

Generally looks good, good work :) will check later in detail, not ideal from the phone

EPashkin added some commits Dec 17, 2017

@EPashkin

This comment has been minimized.

Member

EPashkin commented Dec 17, 2017

Updated.

@EPashkin

This comment has been minimized.

Member

EPashkin commented Dec 28, 2017

@sdroege Please, look at this if you have time.

@sdroege

This comment has been minimized.

Member

sdroege commented Dec 28, 2017

I will tomorrow, thanks for the reminder. I missed that something is left to be done here

@sdroege

This comment has been minimized.

Member

sdroege commented Dec 29, 2017

Also feel free to remind me sooner than ~2 weeks later for such things in the future :)

fn composite_color_simple(&self, dest_width: i32, dest_height: i32, interp_type: InterpType, overall_alpha: i32, check_size: i32, color1: u32, color2: u32) -> Option<Pixbuf>;
fn copy(&self) -> Option<Pixbuf>;

This comment has been minimized.

@sdroege

sdroege Dec 29, 2017

Member

Can this really fail? And the others returning an Option

This comment has been minimized.

@EPashkin

EPashkin Dec 29, 2017

Member

I will check all others later.

This comment has been minimized.

@sdroege

sdroege Dec 29, 2017

Member

Interesting

//#[cfg(any(feature = "v2_36", feature = "dox"))]
//fn save_to_streamv_async<'a, P: IsA</*Ignored*/gio::OutputStream>, Q: Into<Option<&'a gio::Cancellable>>, R: /*Ignored*/gio::AsyncReadyCallback>(&self, stream: &P, type_: &str, option_keys: &[&str], option_values: &[&str], cancellable: Q, callback: R);
//fn save_to_streamv_async<'a, P: IsA<gio::OutputStream>, Q: Into<Option<&'a gio::Cancellable>>, R: /*Unimplemented*/gio::AsyncReadyCallback>(&self, stream: &P, type_: &str, option_keys: &[&str], option_values: &[&str], cancellable: Q, callback: R);
fn savev(&self, filename: &str, type_: &str, option_keys: &[&str], option_values: &[&str]) -> Result<(), Error>;

This comment has been minimized.

@sdroege

sdroege Dec 29, 2017

Member

This was already weird before, but it seems like this should be taking a options: &[(&str, &str)] instead? It's probably unsafe now if the arrays are different length?

This comment has been minimized.

@EPashkin

EPashkin Dec 29, 2017

Member

You right. Maybe even options: &[(AsRef<str>, AsRef<str>)]

@@ -178,10 +163,44 @@ impl Pixbuf {
}
}
pub fn copy(&self) -> Pixbuf {

This comment has been minimized.

@sdroege

sdroege Dec 29, 2017

Member

Here it also was not returning an Option.

Maybe take a diff of the API changes here to make sure the API is not regressing without noticing.

@@ -178,10 +163,44 @@ impl Pixbuf {
}
}
pub fn copy(&self) -> Pixbuf {
pub fn get_file_info(filename: &str) -> Option<(PixbufFormat, i32, i32)> {

This comment has been minimized.

@sdroege

sdroege Dec 29, 2017

Member

This should probably be a &Path?

This comment has been minimized.

@EPashkin

EPashkin Dec 29, 2017

Member

Yes, I change it to AsRef<Path>

@EPashkin

This comment has been minimized.

Member

EPashkin commented Dec 30, 2017

@sdroege Updated with AsRef<Path> for file names and &[(&str, &str)] for save options.
Tried &[(AsRef<str>, AsRef<str>)], but it make problem with saving without options.
About return Option<Pixbuf>: seems Pixbuf even don't follow glib rules that object constructors can't return null, so all function returning Pixbuf need be optional.
I stuck with rust-lang/rust#47048 and can't build gir, so I can't change this now and propose postpone this change.

@sdroege

This comment has been minimized.

Member

sdroege commented Dec 30, 2017

👍

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Dec 30, 2017

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 1e9ce85 into gtk-rs:master Dec 30, 2017

2 checks passed

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

@EPashkin EPashkin deleted the EPashkin:fix_PixbufFormat_regen_Pixbuf branch Dec 30, 2017

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