Re: Built in themed icons

[Owen Taylor <otaylor redhat com>]

On Fri, 2003-05-09 at 20:50, J. Ali Harlow wrote:
> On 2003.05.09 23:00 Owen Taylor wrote:
> 
> >  - Instead of a magic filename "" (that has to be freed like any
> >    other filename), perhaps it would be better to use a magic
> >    value GTK_ICON_THEME_BUILTIN that you compare against with
> >    == and don't free?
> 
> Just on this point - I think this would be a very bad idea. It's
> already a little confusing as to when you should free return
> values in Gtk+; adding a special case where sometimes you free
> and sometimes you don't is just asking for trouble. As it happens
> I don't see that it buys you anything, but even if it did I fear
> it would set a bad precedent.

Well, what it buys you is that you can compare against a constant
value, which is less 'magic' then using "".

But there is certainly a point that it makes things hard on language
bindings.

Thinking about it, really the answer is just to do things right,
instead of hacking around. 


char *gtk_icon_theme_lookup_icon (GtkIconTheme                *theme,
                                  const gchar                 *icon_name,
                                  gint                         size, 
                                  GtkIconLookupFlags           flags,
                                  G_CONST_RETURN GtkIconData **icon_data,
                                  gint                        *base_size);

Should be redone as:

GtkIconInfo *gtk_icon_theme_lookup_icon (GtkIconTheme       *theme
                                         const gchar        *icon_name,
                                         GtkIconLookupFlags  flags);

With GtkIconInfo a boxed type. (copy()/free())

G_CONST_RETURN gchar *gtk_icon_info_get_filename       (GtkIconInfo *info);
GdkPixbuf            *gtk_icon_info_get_builtin_pixbuf (GtkIconInfo *info);
gint                  gtk_icon_info_get_base_size      (GtkIconInfo *info);
 
Plus methods for what is currently in GtkIconData 
(get_attach_points(), get_embedded_rect(), get_display_name()).

That cleans up the API a bunch and improves future extensibility. 

There is some GnomeIconTheme compatibility concern with getting rid of 
GtkIconData; I guess the GnomeIconTheme wrapper can keep around a single
GnomeIconData that it fills in from the GtkIconInfo and returns pointers to.

Regards,
                                                   Owen