Re: [evolution-patches] Icon Theme Patch

[Not Zed <notzed ximian com>]

Shouldn't the size be specified as like
E_ICON_FACTORY_ICON
E_ICON_FACTORY_MENU
etc.
rather than
16
24
32?

I don't think it should specify the exact pixel sizes.

-       iconpart = em_format_html_file_part((EMFormatHTML *)emf, "image/png", EVOLUTION_ICONSDIR,
-                                           comp&&comp[0]?"flag-for-followup-done-16.png":"flag-for-followup-16.png");
-       if (iconpart) {
-               char *classid;
-
-               classid = "" emf->part_id->str, comp&&comp[0]?"comp":"uncomp");
-               camel_stream_printf(stream, "<td align=\"left\"><img src="" classid);
-               (void)em_format_add_puri(emf, sizeof(EMFormatPURI), classid, iconpart, efhd_write_image);
-               g_free(classid);
-               camel_object_unref(iconpart);
+       iconpath = e_icon_factory_get_icon_filename (comp && comp[0] ? "stock_flag-for-followup-done" : "stock_flag-for-followup", 16);
+       if (iconpath) {
+               CamelMimePart *iconpart;
+
+               iconpart = em_format_html_file_part((EMFormatHTML *)emf, "image/png", iconpath);
+               g_free (iconpath);
+               if (iconpart) {
+                       char *classid;
+
+                       classid = "" emf->part_id->str, comp&&comp[0]?"comp":"uncomp");
+                       camel_stream_printf(stream, "<td align=\"left\"><img src="" classid);
+                       (void)em_format_add_puri(emf, sizeof(EMFormatPURI), classid, iconpart, efhd_write_image);
+                       g_free(classid);
+                       camel_object_unref(iconpart);
+               }
        }


For above:
1. we need to display something if we get no filename.  does e-icon-theme fallback to an unthemed icon if it can't find one?  do we have to check for null filename at all?
2. how can we be sure that the icon has a filename, and that it is a png?
3. in the sign icon stuff later on we don't check filename isn't null.

-       char *filename;
+       gchar *basename;
 
use char instead of gchar everywhere in the mail code.

The new code in e-icon-theme.c uses
}
 else {
it should be:
} else {


Also, why are we using pixbufs?  Pixmaps would be far more efficient for most cases.

Allso there is some inconsitency with hyphens.  Should they all be - or _ or why the different mix?

+       icon_list = e_icon_factory_get_icon_list ("stock_mail-send-receive");
+       if (icon_list) {
+               gtk_window_set_icon_list (GTK_WINDOW (gd), icon_list);
+               g_list_foreach (icon_list, (GFunc) g_object_unref, NULL);
+               g_list_free (icon_list);
+       }

this code is repeated a lot, should there be a free_icon_list call?



On Fri, 2004-04-16 at 15:56 -0400, Rodney Dawes wrote:
> So,
>
> Here it finally is. I've gone through and updated the patch to
> apply/build against the latest cvs (as of 3:50 PM EST today, anyway).
> I've also gone through and taken care to fix the issues where the patch
> was requesting the wrong icon names, and the places where we've recently
> changed things that add new widgets that weren't using this stuff. The
> ChangeLog entries contain the mostly appropriate accredidation of either
> Michael Terry, or myself. It also requires gnome-icon-theme 1.2.0 at
> configure time now, since there is a pkgconfig file there. I'd like to
> get this committed as soon as possible. It works great, except for like
> one or two problems in the icon theme itself, where images get scaled
> up. It is particularly noticable in the mailer for the Important column.
> I've filed a bug against gnome-icon-theme upstream for Jakub to fix.
>
> I also know how to do the right thing for fallback icons now, but I want
> to get these bits out of my tree, so I don't have 80+ modified files
> sitting around, waiting to get more conflicts from cvs updates. The
> patch works great for me.
>
> -- dobey

Michael Zucchi <notzed ximian com> Ximian Evolution and Free Software Developer

Novell, Inc.