Re: templates patch ...

[Christian Neumair <cneumair gnome org>]

Dear Michael,

Am Dienstag, den 24.06.2008, 22:08 +0100 schrieb Michael Meeks:
> 	Quick review appreciated for a patch to support system templates
> (from) /usr/share/templates in the same way that KDE does


Thanks for your efforts. I am mostly fine with the patch, but I have a
few minor comments:

How does KDE sort the menu items? IMO it would make sense to merge all
the template sources (including subdirectories), and then sort the items
by display name.

I wonder why you migrated the template parameters to textual URIs. Using
GFiles and NautilusFiles rather than URI strings is a Nautilus
convention we should stick to.

g_str_has_suffix() would also look more readable for detecting a
".desktop" suffix in add_template_to_templates_menus().

I agree that the
g_return_if_fail (nautilus_file_is_local (source));
call should vanish.

Regarding the get_template_source_if_valid(), you should use
g_ascii_strncasecmp() for the desktop file type check, and ensure that
it is not NULL before strcmp'ing it. You also seem to return
the .desktop file's URI if it is not a link, instead of returning NULL.
This looks wrong.

> 	[ Incidentally - if it is more widely used - should it not be one slot
> higher up than "Add Launcher" in the right-click context menu ? ]

You mean it should be at the top, above "New Folder"? I often created
new folders, but almost never new documents.

best regards,
 Christian Neumair

-- 
Christian Neumair <cneumair gnome org>