gtk-rs / gtk3-rs

Rust bindings for GTK 3
https://gtk-rs.org
MIT License
508 stars 90 forks source link

Add binding for gtk_file_chooser_add_choice() #760

Closed kelnos closed 2 years ago

kelnos commented 2 years ago

Looks like binding for this was skipped because the signature is not very Rust-idiomatic.

Two things:

  1. Is adding the manual_traits bit in Gir.toml necessary? It didn't appear to change the output of the codegen when I re-ran it.
  2. Just want to confirm that .to_glib_none() on a Vec will give us a NULL-terminated pointer array. (I think this is the case from looking at to_glib_none_from_slice(), which uses a GPtrArray, but wanted to be sure.)
  3. The add_choice() function signature:
    • Another option I considered was just options: [(&str, &str)] (no Option around it), and just interpret "empty array" as "I want this to be a boolean choice".
    • I like the explicit nature of Some/None there, though. But in practice I think an implementation would have to treat None and &[] as the same, since a combo box with zero items isn't all that useful. Interestingly, I think libgtk just considers passing empty lists as a programmer error, as it doesn't bother to check for that; passing an empty array will just get you an empty combo box, and the only way you get a check box is by passing NULL.
    • So my feeling is that Option<(&str, &str)> is the right way to go here, since it has the same behavior when passed to down to libgtk (even though I don't think that behavior is particularly good or useful when an empty array is passed).
bilelmoussaoui commented 2 years ago

Is adding the manual_traits bit in Gir.toml necessary? It didn't appear to change the output of the codegen when I re-ran it.

It is needed for the generated docs, see the list of "Implements" traits.

So my feeling is that Option<(&str, &str)> is the right way to go here, since it has the same behavior when passed to down to libgtk (even though I don't think that behavior is particularly good or useful when an empty array is passed).

Please see how we have implemented that in gtk4-rs, the corresponding PR should have details on why we decided to not use a Option<&[(&str, &str)]>

kelnos commented 2 years ago

Ah, doh, should have looked at gtk4-rs first to see if this was done already! Probably best that I just copy over your work rather than reinventing.

kelnos commented 2 years ago

Ok, just copied over the gtk4-rs version (and added the feature gate).