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

Generate fundamental types code using gir #780

Merged
merged 12 commits into from
Jan 1, 2022

Conversation

bilelmoussaoui
Copy link
Member

Depends on gtk-rs/gir#1294

Still needs a bit of cleanup & figure out how to handle the builder pattern, but in general it drops a bunch of manual code and that makes me happy :)

@bilelmoussaoui bilelmoussaoui force-pushed the bilelmoussaoui/fundemantal-types branch 8 times, most recently from 01b4a85 to d6d395e Compare December 29, 2021 15:08
Except ExpressionWatch as it has it's get_type function exposed as part of the API
only in 4.2 which can't be handled automatically
until we drop support for 4.0
axis_use.into_glib(),
value.as_mut_ptr(),
));
let value = value.assume_init();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very bad, gir should assume_init only if ret.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@bilelmoussaoui
Copy link
Member Author

Before merging this, we need to check the functions signatures one by one to ensure everything is properly generated & fix whatever needed.

@bilelmoussaoui
Copy link
Member Author

So I did a 1:1 check (manual implementations we had vs auto-generated ones):

Here are a couple of things i noticed

  • gtk_property_expression_get_expression -> nullable : fixed upstream, see https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/4304
  • gsk_container_node_get_child -> assert n_children < i # fixed in the latest commit
  • gsk_gl_shader_node_get_child -> assert n_children < i # fixed in the latest commit

Those that were not nullable in the manual bindings but are in the auto-generated ones. There are a ton of others that should really not be nullable because the constructor doesn't take a nullable variant and are nullable because of #68. So for those I would rather leave them as is for now and tackle the issue on #68

  • gsk_border_node_get_outline -> not nullable
  • gsk_conic_gradient_node_get_center -> not nullable
  • gdk_event_get_event_sequence -> not nullable
  • gsk_linear_gradient_node_get_end -> not nullable
  • gsk_linear_gradient_node_get_start -> not nullable

gdk4/Gir.toml Outdated Show resolved Hide resolved
gsk4/Gir.toml Show resolved Hide resolved
gsk4/Gir.toml Show resolved Hide resolved
@sdroege
Copy link
Member

sdroege commented Jan 1, 2022

Generally looks good to me otherwise

@sdroege
Copy link
Member

sdroege commented Jan 1, 2022

There are a ton of others that should really not be nullable because the constructor doesn't take a nullable variant and are nullable because of #68. So for those I would rather leave them as is for now and tackle the issue on #68

You can configure the return type to be not nullable in the Gir.toml for the time being.

@bilelmoussaoui
Copy link
Member Author

There are a ton of others that should really not be nullable because the constructor doesn't take a nullable variant and are nullable because of #68. So for those I would rather leave them as is for now and tackle the issue on #68

You can configure the return type to be not nullable in the Gir.toml for the time being.

Sure, but most of them were like that. I plan to tackle the nullability issue just after getting those two PR merged so it's fine I guess

@sdroege
Copy link
Member

sdroege commented Jan 1, 2022

Sure, but most of them were like that. I plan to tackle the nullability issue just after getting those two PR merged so it's fine I guess

Ok, let's make sure to fix that before the release though.

@bilelmoussaoui
Copy link
Member Author

Sure, but most of them were like that. I plan to tackle the nullability issue just after getting those two PR merged so it's fine I guess

Ok, let's make sure to fix that before the release though.

Sure thing

@bilelmoussaoui bilelmoussaoui changed the title Generate fundemental types code using gir Generate fundamental types code using gir Jan 1, 2022
@bilelmoussaoui
Copy link
Member Author

Will merge this & rebase the other one once the gir fix lands

@bilelmoussaoui bilelmoussaoui merged commit b1c21a2 into master Jan 1, 2022
@bilelmoussaoui bilelmoussaoui deleted the bilelmoussaoui/fundemantal-types branch January 1, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants