Re: GEP 7 - icons and thumbnails - gnome_icon_loader

Hi Alex,

On Sat, 2002-09-21 at 15:14, Alexander Larsson wrote:
> So, nobody is commenting on this ...

	Ok, I got there in the end; overall it's great. I have a few more
questions and points though. Also, my suggestion to use a requirements
GEP seems particularly pointless here; far better to have the actions
more clearly codified. Would you like me to massage it into a more
reasonable Action form ?

	+ why call it GnomeIconLoader - it does appear to load icons.
	  would GnomeIconFinder be a better name ?

	+ I assume we'd need to include gnome-icon-theme.[ch], is that
	  intended as an API for public consumption ? or will it be 
	  implementation private ?

	+ I'm not quite clear on (theme_lookup_icon):

  if (min_dir)
    {
      suffix = GPOINTER_TO_INT (g_hash_table_lookup (min_dir->icons,
icon_name));
      file = g_strconcat (icon_name, string_from_suffix (suffix), NULL);

	What happens if there is no icon of name 'icon_name' in min_dir?
do we need to detect that before assert failure in string_from_suffix ?
or is that not possible ?

	+ I assume this rids us of the theme's extra XML description
	  file containing the attach points, unifying that into the 
	  theme file ? - it seems you bin the per size attach points
	  too which is great :-)

	+ I'm slightly concerned about the attach points / text 
	  rectangle it looks like you're specifying the points as
	  guint16's - why ? is that a fixed point fraction of the
	  icon's size ? - or it's ultimate bounding box ? Similarly
	  for the embedded text stuff. Of course - quite possibly since
	  you're returning pixmaps of a certain size that's no problem,
	  but we need some sane way to normalise this on disk so it can
	  be scaled [esp. for SVG] - or perhaps I lost it again ? ;-)

	+ I assume that there will be very few GnomeIconData records
	  hanging around in memory of the order of ~2, is that so ?

	+ Unsure what theme->display_name, theme->comment are for, is it
	  exposed, looks like they are read, stored, destroyed without
          use ? similarly, what is the GnomeIconData->display_name for ?
	  and if neither of these are that useful - do we need the
	  localization burden of "Yet another file to localise" :-) at
	  least with XML files we have existing tools. So ... in 
	  summary:

	+ is gnome_theme_file_get_locale_string neccessary ? and what
	  i18ntools work / autotools burden does this create ?

	+ my_g_str_has_suffix: presumably this can be axed in favour of
	  the version in glib ?

	+ What is the thinking behind 'rescan_if_needed' ? when would
	  that be invoked ? idly / timeout - in case someone added new
	  icons to a theme, without poking the gconf key ?

	It'd be nice to see an example working icon setup, perhaps I missed one
in nautilus ?

	Overall - it's really good it seems to me;

	HTH,

		Michael.

-- 
 [email protected]  <><, Pseudo Engineer, itinerant idiot