Skip to content

Don't skip Option for nullable collections#1728

Closed
fengalin wants to merge 4 commits into
gtk-rs:mainfrom
fengalin:nullable-arrays
Closed

Don't skip Option for nullable collections#1728
fengalin wants to merge 4 commits into
gtk-rs:mainfrom
fengalin:nullable-arrays

Conversation

@fengalin

Copy link
Copy Markdown
Contributor

Consequence of gtk-rs/gir#1622

Comment thread gdk-pixbuf/Gir.toml
Comment thread gio/src/auto/functions.rs Outdated
pub fn content_type_guess(
filename: Option<impl AsRef<std::path::Path>>,
data: &[u8],
data: Option<&[u8]>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs rebasing as this is now manually implemented. Can be autogenerated again :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased, but the manual impl has been reverted: #1720

Note that was the whole point all of this, so I wonder if we haven't lost our time...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it was reverted because it is an api break

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It was only reverted in the 0.20 branch. main still has it and should have it with the Option.

@fengalin fengalin force-pushed the nullable-arrays branch 2 times, most recently from 5e597f9 to 66a0c62 Compare May 27, 2025 09:04
@sdroege sdroege added this to the 0.22 milestone Jul 15, 2025
@fengalin fengalin force-pushed the nullable-arrays branch 2 times, most recently from fe27928 to 147e779 Compare September 5, 2025 16:22
@fengalin

fengalin commented Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Resumed this. Had to implement new traits for the containers. Will perform a sanity check later as this was tedious and error prone.

Comment thread gdk-pixbuf/src/auto/pixbuf_format.rs Outdated
pub fn extensions(&self) -> Option<Vec<glib::GString>> {
unsafe {
FromGlibPtrContainer::from_glib_full(ffi::gdk_pixbuf_format_get_extensions(
TryFromGlibPtrContainer::try_from_glib_full(ffi::gdk_pixbuf_format_get_extensions(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would instead name it, MaybeFromGlibPtrContainer or so. Try as a prefix assumes we would get a Result back instead of an Option

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am also not sure if all those returned Option<Vec<T>> make sense or some of them needs annotations fixes to return just Vec<T> that would be empty if there are no items.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would instead name it, MaybeFromGlibPtrContainer or so. Try as a prefix assumes we would get a Result back instead of an Option

Indeed. done.

I am also not sure if all those returned Option<Vec> make sense or some of them needs annotations fixes to return just Vec that would be empty if there are no items.

Yes, I went looking at the documentation for these functions and spotted some discrepancies last time I tackled this. I'll force annotations with my conclusions in a bit. I just wanted to rebase & push now that I have something usable as a whole (last time I paused, I had just figured out I had to implement all those Maybe traits).

@fengalin fengalin force-pushed the nullable-arrays branch 3 times, most recently from cce0fa9 to 2c9291c Compare September 8, 2025 10:49
C containers can be NULL, which is represented by None in Rust.

This commit:

* adds container specific `MaybeFromGlib` traits which return an Option,
* implements them for the existing containers,
* base the container specific `FromGlib` implementations off the `MaybeFromGlib`
  implementations.
@fengalin fengalin force-pushed the nullable-arrays branch 2 times, most recently from e4dad15 to 038bccc Compare September 9, 2025 16:37
* [`gdk_pixbuf_format_get_extensions`]
  > The array is `NULL`-terminated.
* [`gdk_pixbuf_format_get_mime_types`]
  > The array is `NULL`-terminated.
* [`g_dbus_proxy_get_cached_property_names`]
  > [...] `NULL` if the value is not in the cache.
* [`g_file_info_list_attributes`]
  > [...] `NULL` on error.
  ==> but NULL as an error is only used for the case where `FileInfo` is NULL, which is not possible in Rust.
* [`g_tls_certificate_get_dns_names`]
  > [...] `NULL` if it’s not available.
* [`g_tls_certificate_get_ip_addresses`]
  > [...] `NULL` if it’s not available.
* [`pango_font_get_languages`]
  > The array is `NULL`-terminated.
* [`pango_font_face_list_sizes`]
  > This is only applicable to bitmap fonts. For scalable fonts, stores `NULL` at the location pointed to by sizes and `0` at the location pointed to by `n_sizes`.
* [`pango_language_get_preferred`](https://docs.gtk.org/Pango/type_func.Language.get_preferred.html)
  > The array is `NULL`-terminated.

[`gdk_pixbuf_format_get_extensions`]: https://docs.gtk.org/gdk-pixbuf/method.PixbufFormat.get_extensions.html
[`gdk_pixbuf_format_get_mime_types`]: https://docs.gtk.org/gdk-pixbuf/method.PixbufFormat.get_mime_types.html
[`g_dbus_proxy_get_cached_property_names`]: https://docs.gtk.org/gio/method.DBusProxy.get_cached_property.html
[`g_file_info_list_attributes`]: https://docs.gtk.org/gio/method.FileInfo.list_attributes.html
[`g_tls_certificate_get_dns_names`]: https://docs.gtk.org/gio/method.TlsCertificate.get_dns_names.html
[`g_tls_certificate_get_ip_addresses`]: https://docs.gtk.org/gio/method.TlsCertificate.get_ip_addresses.html
[`pango_font_get_languages`]: https://docs.gtk.org/Pango/method.Font.get_languages.html
[`pango_font_face_list_sizes`]: https://docs.gtk.org/Pango/method.FontFace.list_sizes.html
[`pango_language_get_preferred`]: https://docs.gtk.org/Pango/type_func.Language.get_preferred.html
@fengalin

fengalin commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

Postponing these changes to a future Rust bindings generator rewrite

@fengalin fengalin closed this May 4, 2026
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.

3 participants