Created attachment 278238 [details] [review]
nautilus extension patch

Review of attachment 278238 [details] [review]:

Thanks for the bug report and patch!

For the Makefile changes, please keep everything in the toplevel Makefile.am. Also, it looks like you are missing the necessary Makefile changes from this patch.

For the source code, please try to use the coding style described in the HACKING file. Roughly, the convention is 4-space indentation, with a BSD/Allman brace style:

https://git.gnome.org/browse/easytag/tree/HACKING

You can look at recent EasyTAG commits to get a better feeling for the coding style.

::: configure.ac
@@ +302,3 @@
+dnl # Nautilus
+dnl ################################################
+if test x"$enable_nautilus_actions" != x"no" ; then

Please use AS_IF instead of shell if here.

@@ +303,3 @@
+dnl ################################################
+if test x"$enable_nautilus_actions" != x"no" ; then
+  PKG_CHECK_MODULES(NAUTILUS,         \

You do not need the trailing slash here.

@@ +304,3 @@
+if test x"$enable_nautilus_actions" != x"no" ; then
+  PKG_CHECK_MODULES(NAUTILUS,         \
+                    [libnautilus-extension glib-2.0 gio-2.0],

You only need to check for libnautilus-extension, as glib-2.0 and gio-2.0 are in the Requires line of the extension .pc file. However, you should note that (from the GDesktopAppInfo documentation) "…<gio/gdesktopappinfo.h> belongs to the UNIX-specific GIO interfaces, thus you have to use the gio-unix-2.0.pc pkg-config file when using it.".

@@ +312,3 @@
+AC_SUBST(NAUTILUS_CFLAGS)
+AC_SUBST(NAUTILUS_LIBS)
+AC_SUBST(NAUTILUS_EXTENSION_DIR)

You only need to substitute the extension directory, as the CFLAGS and LIBS variables are automatically substituted with pkg-config 0.24 and above.

::: nautilus/nautilus-easytag.c
@@ +41,3 @@
+  dir = g_object_get_data (G_OBJECT (item), "dir");
+
+  appinfo = g_desktop_app_info_new_from_filename ("/usr/share/applications/easytag.desktop");

This does not take into account desktop files installed in any of the other XDG directories, such as the user's home directory. You should use g_desktop_app_info_new() instead, with just the desktop filename.

@@ +64,3 @@
+  files = g_object_get_data (G_OBJECT (item), "files");
+  
+  appinfo = g_desktop_app_info_new_from_filename ("/usr/share/applications/easytag.desktop");

Same comment here regarding g_desktop_app_info_new().

@@ +104,3 @@
+{
+  FileMimeInfo file_mime_info;
+  int          i;

Use a gsize here.

@@ +132,3 @@
+  if (scheme != NULL) {
+    const char *unsupported[] = { "trash", "computer", NULL };
+    int         i;

Array lengths should use a gsize.

@@ +182,3 @@
+    item = nautilus_menu_item_new ("NautilusEasyTag::open_directory",
+                                   "Open in EasyTag",
+                                   "Open the current selected directory on EasyTag",

These strings are user-visible, and should be translatable. In this case, I think that wrapping the strings in _(), listing the source file in po/POTFILES.in and including <glib/gi18n-lib.h> should work.

@@ +198,3 @@
+
+    item = nautilus_menu_item_new ("NautilusEasyTag::open_files",
+                                   "Open with EasyTag...",

The "..." (three full stops) should be replaced with an ellipsis "…".

@@ +199,3 @@
+    item = nautilus_menu_item_new ("NautilusEasyTag::open_files",
+                                   "Open with EasyTag...",
+                                   "Open selected files in EasyTag",

Translatable strings here too.

::: nautilus/nautilus-easytag.h
@@ +33,3 @@
+
+struct _NautilusEasyTag {
+  GObject __parent;

In C, symbols starting with a double underscore are reserved for use by the implementation, so you should remove the underscores here and elsewhere.

@@ +37,3 @@
+
+struct _NautilusEasyTagClass {
+  GObjectClass __parent;

Leading double underscore.

Created attachment 278279 [details] [review]
fixed extension

Comment on attachment 278279 [details] [review]
fixed extension

Thanks for the updated patch. I made a few more changes, and pushed it to master as 145685dbdc6c5ab6654600b908c272a3829863bb. I pushed some follow-up commits and tested the extension, and it seems to work fine. Thanks again!