Re: GEP 7 - icons and thumbnails - gnome-thumbnail

[Michael Meeks <michael ximian com>]

Hi Alex,

	Just poking at gnome-thumbnail; looks nice - especially since it has
lots of API documentation ( lacking in GnomeIconLoader ).

	+ gnome_thumbnail_load_pixbuf looks like a useful API to have,
	  but a shame to put it in the thumbnail namespace; is there
	  somewhere better for it ? 'gnome_pixbuf_new_from_uri ()' ?
	  not clear what to do.

	+ How useful is gnome_thumbnail_has_uri ? it looks somewhat 
	  flakey to me; only a getter, returning only a boolean ? and
	  looks like it segv's if the option doesn't exist. [ I see
	  this is always set for the thumbnails we generate but ... ]

	+ Similar issue in gnome_thumbnail_is_valid; that is unless
	  pixbuf_get_option always returns a valid string - 2 segvs 
	  there.

	+ it's interesting to use an md5 sum of the URI to form the
	  path to the file - I suppose to avoid path / filename length
	  limits yes ?

	+ the magic 'mimetype_supported_by_gdk_pixbuf' thing is as nasty
	  as it ever was ;-) hard to know how to fix it efficiently, 
	  without yet more evil config file.

	+ the mkstemp man page says:

		The  old behaviour (creating a file with mode 0666) may
	        be a security risk, especially since other Unix flavours
		use 0600, and somebody might overlook this detail when
		porting programs.

	  do we need a chmod there ?

	+ The gnome_thumbnail_factory_generate_thumbnail call is great,
	  but I think people will use it ;-) would it be better to 
	  expose an 'async' flavour, that spawned a thread to generate
	  the thumb ? or does that introduce the headache of thread
	  queues and job scheduling into the issue ? - unnecessarily 
	  painful. [ could use a GThreadPool argument ] - anyway that's
	  future work.

	+ What is the hard-coded "128" in expand_thumbnailing_script ?
	  is that intended to be a %d, size passed in from 
	  generate_thumbnail ?

	Overall, looks really excellent, good work !

	Regards,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot