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

Generate functions that uses gsize/gusize #183

Merged
merged 1 commit into from Jun 19, 2017

Conversation

Projects
None yet
3 participants
@EPashkin
Member

EPashkin commented Jun 18, 2017

Part of gtk-rs/gir#385

Only ignored functions that causes compiler errors

cc @sdroege

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2017

Member

@GuillaumeGomez, take look at this. CI passed.

Member

EPashkin commented Jun 18, 2017

@GuillaumeGomez, take look at this. CI passed.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2017

Member

Ignored functions, added comment #gsize.

Member

EPashkin commented Jun 18, 2017

Ignored functions, added comment #gsize.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jun 18, 2017

Member

👍

Member

sdroege commented Jun 18, 2017

👍

//pub fn to_data(&self) -> Result<(String, /*Unimplemented*/Fundamental: Size), Error> {
// unsafe { TODO: call ffi::g_key_file_to_data() }
//}
pub fn to_data(&self) -> Result<(String, usize), Error> {

This comment has been minimized.

@sdroege

sdroege Jun 18, 2017

Member

This one is also suboptimal. The usize is the length of the string, it should just return Result<String, Error>. Also going to be fixed with my array-length changes.

But this one here is at least not unsafe, only suboptimal :)

@sdroege

sdroege Jun 18, 2017

Member

This one is also suboptimal. The usize is the length of the string, it should just return Result<String, Error>. Also going to be fixed with my array-length changes.

But this one here is at least not unsafe, only suboptimal :)

This comment has been minimized.

@EPashkin

EPashkin Jun 19, 2017

Member

Yes, that why I don't ignore it and other returning functions.

@EPashkin

EPashkin Jun 19, 2017

Member

Yes, that why I don't ignore it and other returning functions.

This comment has been minimized.

@sdroege

sdroege Jun 19, 2017

Member

Yeah, just saying :) In my local branch this ends up being pub fn to_data(&self) -> Result<String, Error> already btw. Hope to be able to clean that up a bit for an initial review later today.

@sdroege

sdroege Jun 19, 2017

Member

Yeah, just saying :) In my local branch this ends up being pub fn to_data(&self) -> Result<String, Error> already btw. Hope to be able to clean that up a bit for an initial review later today.

This comment has been minimized.

@sdroege

sdroege Jun 19, 2017

Member

This one actually not because it misses the array-length annotation for the return value in GLib. We should go through all these things and report them in Bugzilla...

@sdroege

sdroege Jun 19, 2017

Member

This one actually not because it misses the array-length annotation for the return value in GLib. We should go through all these things and report them in Bugzilla...

//pub fn get_string_list(&self, group_name: &str, key: &str) -> Result<(Vec<String>, /*Unimplemented*/Fundamental: Size), Error> {
// unsafe { TODO: call ffi::g_key_file_get_string_list() }
//}
pub fn get_string_list(&self, group_name: &str, key: &str) -> Result<(Vec<String>, usize), Error> {

This comment has been minimized.

@sdroege

sdroege Jun 18, 2017

Member

Same as g_key_file_to_data(), and also all the other non-ignored functions

@sdroege

sdroege Jun 18, 2017

Member

Same as g_key_file_to_data(), and also all the other non-ignored functions

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jun 19, 2017

Member

I'm fine with this and @sdroege seems to be fine too so let's merge. Thanks!

Member

GuillaumeGomez commented Jun 19, 2017

I'm fine with this and @sdroege seems to be fine too so let's merge. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 46658ce into gtk-rs:master Jun 19, 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:gsize branch Jun 19, 2017

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 25, 2017

Member

Opps, forgot to check git submodule version: it stay old, in other PR's seems too.

Member

EPashkin commented Jun 25, 2017

Opps, forgot to check git submodule version: it stay old, in other PR's seems too.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jun 25, 2017

Member

Oups, forgot to check as well. Good catch @EPashkin!

Member

GuillaumeGomez commented Jun 25, 2017

Oups, forgot to check as well. Good catch @EPashkin!

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