gtk-rs / gtk-rs-core

Rust bindings for GNOME libraries
https://gtk-rs.org/gtk-rs-core
MIT License
272 stars 105 forks source link

[FEATURE REQUEST] atomic gobject_set_properties #1339

Closed emmanueltouzery closed 3 months ago

emmanueltouzery commented 4 months ago

glib::ObjectExt::set_properties sets all properties one by one in a loop: https://github.com/gtk-rs/gtk-rs-core/blob/master/glib/src/object.rs#L2290

however I would have expected it to behave the same as the GObject set: https://docs.gtk.org/gobject/method.Object.set.html

Where it's written:

Note that the “notify” signals are queued and only emitted (in reverse order) after all properties have been set. See g_object_freeze_notify().

As it is with the gtkrs function, there is no queueing of notify signals, the first signal is sent when only the first property was set. I think gtk-rs should use freeze_notify in this function to achieve that effect.

sdroege commented 3 months ago

That sounds like a good idea. Do you want to provide a PR for this?

emmanueltouzery commented 3 months ago

It really should be almost zero code (literally 1 loc plus the apidoc), but i'd have to at least run it. The issue is that currently the gtkrs4 app i'm writing is still one gtkrs version behind, and i'm not planning to port it to the latest version yet. I'd rather not code it "blind", although presumably it really should work...

sdroege commented 3 months ago

Yes, it's not hard to add and should indeed be a one-liner :) If you prefer I can also add it but it's really just like you say, nothing else is needed.

emmanueltouzery commented 3 months ago

Ok, I'll send the PR, just be aware that I do it 'blind' (just checking that it compiles, and knowing that I made the same change in my app). But yes with your review, it should work just fine.

sdroege commented 3 months ago

See https://github.com/gtk-rs/gtk-rs-core/pull/1355

emmanueltouzery commented 3 months ago

thank you! i just could not make the time for that... i was going to do it eventually.. i think :blush:, anyway thank you!