GNOME Bugzilla – Bug 517676
g_themed_icon_new*() do more than call g_object_new().
Last modified: 2008-03-10 16:08:51 UTC
g_themed_icon_new() and g_themed_icon_new_with_fallbacks() do quite a bit more than just call g_object_new(): http://svn.gnome.org/viewvc/glib/trunk/gio/gthemedicon.c?revision=6336&view=markup Generally, *_new() functions are meant to be just C convenience methods. Language bindings generally just call g_object_new() with the appropriate properties, so they can actually instantiate derived GTypes (allowing them to override default signal handlers and vfuncs). If it's too difficult to do this via properties then additional g_themed_icon_construct(GThemedIcon* self, ) and g_themed_icon_construct_with_fallbacks(GThemedIcon* self, ) functions would be helpful. These would be called by g_themed_icon_new() and g_themed_icon_new_with_fallbacks().
I don't have any objections if someone wants to write a patch for a "names" property. _with_fallbacks() will still have to do some extra work...
Created attachment 106523 [details] [review] attempt at a patch Added three properties "names", "name", and "use-default-fallbacks", which are all construction only, and the last two can only written. I changed g_themed_icon_new_from_names() to only take one parameter (dropped len), requiring the array to be NULL-terminated instead, and updated the only user. However, not sure if this could break anything else...
> I changed g_themed_icon_new_from_names() to only take one parameter Too late for that now, since we are very close to 2.16. What you can do is make the function accept -1 to mean NULL-terminated. If you make use-default-fallbacks an independent property, the code should do the right thing for the combination of names+use-default-fallbacks, too. The current implementation silently assumes that there is only one name if use-default-fallbacks is true. The new properties need doc comments and translated nicks and blurbs.
Also, g_themed_icon_append_name kinda indicates that names is not construct-only - g_themed_icon_append_name needs to emit property change notification at least. But maybe that function is just a bad idea...it also has interesting implications for the use of g_icon_hash
Yeah. I'm not sure g_themed_icon_append_name() was a great idea either. It makes more sense for GIcons to be read only after construction. But it does make some operations easier.
Created attachment 106800 [details] [review] new patch with docs, property notification and the len parameter added back > > I changed g_themed_icon_new_from_names() to only take one parameter > > Too late for that now, since we are very close to 2.16. What you can do is make > the function accept -1 to mean NULL-terminated. Changed it back. > If you make use-default-fallbacks an independent property, the code should do > the right thing for the combination of names+use-default-fallbacks, too. The > current implementation silently assumes that there is only one name if > use-default-fallbacks is true. I thought that this would be better, unless you want it to check every item in the names list, create the fallbacks and then remove duplicates. I thought that this might be overkill. I added a note in the blurb and comments...
Created attachment 106801 [details] [review] patch from the right directory Sorry, ignore that last one.
2008-03-10 Matthias Clasen <mclasen@redhat.com> * gthemedicon.c: Add properties to make bindings happy. (#517676, Samuel Cormier-Iijima)