I don't have a strong oppinion about this, but since it's more or less still possible the break the api (xfconf_g_property_unbind is still unused atm)... i want to have some response to this: Is there is reason the binding api doesn't look like this: XfconfGBinding *xfconf_g_property_bind(XfconfChannel *channel, const gchar *xfconf_property, GType xfconf_property_type, gpointer object, const gchar *object_property); void xfconf_g_property_unbind (XfconfGBinding *binding); void xfconf_g_property_unbind_all(gpointer object); XfconfGBinding *xfconf_g_property_bind_gdkcolor(XfconfChannel *channel, const gchar *xfconf_property, gpointer object, const gchar *object_property); This makes sence, because you can easily store a binding's pointer in a list for unbinding it later in the process. You could also drop the dupplicates of xfconf_property and object_property, since you can work with pspecs internally. Other question could be why the object is a gpointer and not a GObject, it only works with objects right? Not looking for a flamewar here ^_^, just asking.
Err, sorry, not dropping xfconf_property, only the object_property...
There's no reason to expose XfconfGBinding in the API -- it's an implementation detail. Is there a problem with the current implementation? Do you see it behaving incorrectly as-is? The gpointer instead of GObject thing was requested by Jannis -- mainly for convenience to get rid of G_OBJECT() casts all over the place (sorta similar to the prototypes of g_signal_connect() and friends). Unless there's actually a functionality issue here, I'm inclined to leave this alone. We shouldn't be making unnecessary changes, and if it's just a stylistic issue, I'm happy with it how it is.
Well as I said, there is currently no way of 'storing' the binding (in a GList), wich is for example possible with the exo bindings.
(In reply to comment #3) > Well as I said, there is currently no way of 'storing' the binding (in a > GList), wich is for example possible with the exo bindings. But why do you need to? I'm not seeing the use case...
Because if you want to unbind a property, you now have to keep track of (almost) all the data you've send to xfconf_g_property_bind(). A pointer to the XfconfGBinding struct if much easier to store.
Mmm, true. But I'd rather not change the API/ABI just for a little bit of convenience. There's also a question of XfconfGBinding lifetime. If we return that struct to the caller, it's a little weird to say: "The returned struct can be used to remove the binding, but if the XfconfChannel is destroyed, or the bound GObject is destroyed, the XfconfGBinding will be freed and will no longer be valid." It's just a potential source of bugs -- the same pointer address for a freed XfconfGBinding could easily get re-used on the next memory allocation, and if the caller doesn't keep track of binding lifetimes, it'll cause crashes. I guess just returning a guint identifier like g_signal_connect() does would fix that, though. Dunno, want to think about this a bit more.
Well that's also how ExoBindings work, but I guess we can 'assume' you know what you do when you're unbinding...
Ok, I think I'm gonna do this: gulong xfconf_g_property_bind(XfconfChannel *channel, const gchar *xfconf_property, GType xfconf_property_type, gpointer object, const gchar *object_property); gulong xfconf_g_property_bind_gdkcolor(XfconfChannel *channel, const gchar *xfconf_property, gpointer object, const gchar *object_property); void xfconf_g_property_unbind(gulong id); void xfconf_g_property_unbind_by_property(XfconfChannel *channel, const gchar *xfconf_property, gpointer object, const gchar *object_property); This look ok? Also, xfconf_g_property_unbind_all() -- does this seem more useful to take an XfconfChannel and remove all bindings to that channel, or a GObject and remove all bindings to that object? I'd rather leave it as an object, since that means not rearranging some code in xfconf-binding.c, but is that really more useful?
(In reply to comment #8) > Ok, I think I'm gonna do this: > > gulong xfconf_g_property_bind(XfconfChannel *channel, > const gchar *xfconf_property, > GType xfconf_property_type, > gpointer object, > const gchar *object_property); > > gulong xfconf_g_property_bind_gdkcolor(XfconfChannel *channel, > const gchar *xfconf_property, > gpointer object, > const gchar *object_property); > > void xfconf_g_property_unbind(gulong id); > > void xfconf_g_property_unbind_by_property(XfconfChannel *channel, > const gchar *xfconf_property, > gpointer object, > const gchar *object_property); > > This look ok? I'd still choose for a struct, since it's easier to implement and handle for developers. You still have to lookup the id (or do you use g_signal_handler_disconnect() internally?). If you're afraid of gslice craches, then there is a lot to improve in c anyway and for the moment xfconf_g_property_unbind() is not used by us (so far), so the usage is low. Anyway, it's still a lot better then the current api, so if you think this is the way to go, you've got my vote. > Also, xfconf_g_property_unbind_all() -- does this seem more useful to take an > XfconfChannel and remove all bindings to that channel, or a GObject and remove > all bindings to that object? I'd rather leave it as an object, since that > means not rearranging some code in xfconf-binding.c, but is that really more > useful? Well personally i don't see a reason to unbind everything from a channel, but you could do if(G_IS_OBJECT(pointer){...} else if(XFCONF_IS_CHANNEL(pointer){...} internally. Then you don't have to change the api.
Checked in last week...