Merge lp:~victored/pantheon-files/contractor-plugin into lp:~elementary-apps/pantheon-files/trunk

[Victor Martinez]

- Rewrite contractor plugin to use new API. This talks to Contractor by using Granite's Proxy API.
- Drop extended-actions plugin. Same API as previous Contractor.

[lp:~victored/pantheon-files/contractor-plugin]

1160. By Victor Martinez on 2013-04-23

Destroy plugin menu items properly.

This implied some minor changes to the core library. Special care was taken to avoid breaking the existing plugin ABI.

[Sergey "Shnatsel" Davidoff (shnatsel)]

I'm afraid the recent commit didn't help resolve the bug mentioned in https://code.launchpad.net/~victored/granite/contractor-wrapper/+merge/159948/comments/353537 at all

[Sergey "Shnatsel" Davidoff (shnatsel)]

Also, the right-click menu should list contract *names* and show descriptions only as tooltips on them.

review: Needs Fixing

[Sergey "Shnatsel" Davidoff (shnatsel)]

The old implementation showed descriptions only because names were purely technical, the only human-readable thing was description.

[Sergey "Shnatsel" Davidoff (shnatsel)]

And it gets worse! This is where it may end up: http://pix.toile-libre.org/upload/original/1366754094.png

[lp:~victored/pantheon-files/contractor-plugin]

1161. By Victor Martinez on 2013-04-24

Fix bug involving duplicate menu entries.

The code was assuming that the menu passed to the plugin was the same the plugins were added to initially, which is incorrect.

[Victor Martinez (victored)]

Hey Sergey, thank you for testing!

I'm no longer able to reproduce the bug here.

[Sergey "Shnatsel" Davidoff (shnatsel)]

Works properly for me too.

review: Approve

=== modified file 'libcore/PluginManager.vala'
--- libcore/PluginManager.vala	2012-11-06 19:52:04 +0000
+++ libcore/PluginManager.vala	2013-04-24 17:03:26 +0000
@@ -30,7 +30,11 @@
     string plugin_dir;
     Gee.List<string> names;
     bool in_available = false;
-    public GLib.List<Gtk.Widget>? menus;
+
+    [Deprecated (replacement = "Marlin.PluginManager.menuitem_references")]
+    public GLib.List<Gtk.Widget>? menus; // this doesn't manage GObject references properly
+
+    public Gee.List<Gtk.Widget> menuitem_references { get; private set; }
 
     public PluginManager(Settings settings, string field, string plugin_dir)
     {
@@ -39,6 +43,8 @@
         this.plugin_dir = plugin_dir;
         plugin_hash = new Gee.HashMap<string,Plugins.Base>();
         names = new Gee.ArrayList<string>();
+
+        menuitem_references = new Gee.LinkedList<Gtk.Widget> ();
     }
     
     public void load_plugins()
@@ -147,10 +153,36 @@
     
     public void hook_context_menu(Gtk.Widget menu, List<GOF.File> files)
     {
+        drop_menu_references (menu);
+
+        if (menu is Gtk.Menu)
+            drop_plugin_menuitems (menu as Gtk.Menu);
+
+
+        foreach (var plugin in plugin_hash.values)
+            plugin.context_menu (menu, files);
+    }
+
+    private void drop_plugin_menuitems (Gtk.Menu menu) {
+        var plugin_menu = menu as Gtk.Menu;
+
+        assert (plugin_menu != null);
+
+        foreach (var menu_item in menuitem_references)
+            menu_item.parent.remove (menu_item);
+
+        menuitem_references.clear ();
+    }
+
+    [Deprecated (replacement = "Marlin.PluginManager.drop_plugin_menuitems")]
+    private void drop_menu_references (Gtk.Widget menu) {
+        if (menus == null)
+            return;
+
         foreach (var item in menus)
             item.destroy ();
+
         menus = null;
-        foreach(var plugin in plugin_hash.values) plugin.context_menu (menu, files);
     }
     
     public void ui(Gtk.UIManager data)

=== modified file 'plugins/CMakeLists.txt'
--- plugins/CMakeLists.txt	2012-06-01 21:34:30 +0000
+++ plugins/CMakeLists.txt	2013-04-24 17:03:26 +0000
@@ -1,4 +1,3 @@
-add_subdirectory(extended-actions)
 add_subdirectory(contractor)
 add_subdirectory(marlin-trash)
 add_subdirectory(marlin-ctags)

=== modified file 'plugins/contractor/CMakeLists.txt'
--- plugins/contractor/CMakeLists.txt	2013-04-22 07:45:04 +0000
+++ plugins/contractor/CMakeLists.txt	2013-04-24 17:03:26 +0000
@@ -2,7 +2,8 @@
 pkg_check_modules(DEPS REQUIRED
 	gtk+-3.0
     gee-1.0
-    glib-2.0)
+    glib-2.0
+    granite)
 set(CFLAGS
 	${DEPS_CFLAGS} ${DEPS_CFLAGS_OTHER}
 )
@@ -19,6 +20,7 @@
 PACKAGES
     gtk+-3.0
     gee-1.0
+    granite
 OPTIONS
     --thread)
 add_library(marlin-contractor SHARED

=== modified file 'plugins/contractor/plugin.vala'
--- plugins/contractor/plugin.vala	2012-06-02 15:49:15 +0000
+++ plugins/contractor/plugin.vala	2013-04-24 17:03:26 +0000
@@ -2,8 +2,10 @@
  * Authors:
  *      Lucas Baudin <[e-mail]>
  *      ammonkey <[e-mail]>
- *      
+ *      Victor Martinez <[e-mail]>
+ *
  * Copyright (C) Lucas Baudin 2011 <[e-mail]>
+ * Copyright (C) 2013 elementary
  * 
  * Marlin is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -19,138 +21,122 @@
  * with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-using Gtk;
-using Gee;
-
-[DBus (name = "org.elementary.contractor")]
-public interface ContractorService : Object
-{
-    //public abstract GLib.HashTable<string,string>[] GetServicesByMime (string mime) throws IOError;
-    public abstract GLib.HashTable<string,string>[] GetServicesByLocation (string strlocation, string? file_mime="")    throws IOError;
-    public abstract GLib.HashTable<string,string>[] GetServicesByLocationsList (GLib.HashTable<string, string>[] locations)  throws IOError;
+public class Marlin.Plugins.ContractMenuItem : Gtk.MenuItem {
+    private Granite.Services.Contract contract;
+    private File[] files;
+
+    public ContractMenuItem (Granite.Services.Contract contract, File[] files) {
+        this.contract = contract;
+        this.files = files;
+
+        label = contract.get_display_name ();
+        tooltip_text = contract.get_description ();
+    }
+
+    public override void activate () {
+        try {
+            if (files.length == 1)
+                contract.execute_with_file (files[0]);
+            else
+                contract.execute_with_files (files);
+        } catch (Error err) {
+            warning (err.message);
+        }
+    }
 }
 
-public class Marlin.Plugins.Contractor : Marlin.Plugins.Base
-{
-    UIManager ui_manager;
-    Gtk.Menu menu;
-    GOF.File current_directory = null;
-    unowned GLib.List<GOF.File> selection;
-    GLib.HashTable<string,string>[] services = null;
-    
-    private ContractorService service_eactions;
-    
-    public Contractor ()
-    {
-        try {
-            service_eactions = Bus.get_proxy_sync (BusType.SESSION,
-                                                   "org.elementary.contractor",
-                                                   "/org/elementary/contractor");
-        } catch (IOError e) {
-            stderr.printf ("%s\n", e.message);
-        }
-    }
-
-    private string get_app_display_name (GLib.HashTable<string,string> app__)
-    {
-        return app__.lookup ("Description");
-    }
-
-    private GLib.HashTable<string,string> add_location_entry (GOF.File file)
-    {
-        GLib.HashTable<string,string> entry;
-                
-        entry = new GLib.HashTable<string,string> (str_hash, str_equal);
-        entry.insert ("uri", file.uri);
-        var ftype = file.get_ftype ();
-        if (ftype == "application/octet-stream" || ftype == null)
-            entry.insert ("mimetype", "");
-        else
-            entry.insert ("mimetype", ftype);
-
-        return entry;
-    }
-
-    private GLib.HashTable<string, string>[] build_hash_from_list_selection ()
-    {
-        GLib.HashTable<string,string>[] locations = null;
-
-        foreach (GOF.File file in selection) {
-            if (file != null)
-                locations += add_location_entry (file);
-            //message ("file %s", file.name);
-        }
-        if (selection == null && current_directory != null) {
-            locations += add_location_entry (current_directory);
-        }
-
-        return locations;
-    }
-
-    public void action_activated ()
-    {
-        Gtk.MenuItem menuitem;
-        GLib.HashTable<string,string> app__;
-                
-        menuitem = (Gtk.MenuItem) menu.get_active ();
-        app__ = menuitem.get_data<GLib.HashTable<string,string>> ("app");
-
-        if (app__ != null) {
-            var cmd = app__.lookup ("Exec");
-            //message ("test exec %s", cmd);
-            try {
-                GLib.Process.spawn_command_line_async (cmd);
-            } catch (SpawnError e) {
-                stderr.printf ("error spawn command line %s: %s", cmd, e.message);
-            }
-        }
-    }
-
-    public override void context_menu (Gtk.Widget? widget, GLib.List<GOF.File> files)
-    {
+public class Marlin.Plugins.Contractor : Marlin.Plugins.Base {
+    private Gtk.UIManager ui_manager;
+    private Gtk.Menu menu;
+    private GOF.File current_directory = null;
+
+    public Contractor () {
+    }
+
+    public override void context_menu (Gtk.Widget? widget, List<GOF.File> gof_files) {
         menu = widget as Gtk.Menu;
-        
-        selection = files;
+        return_if_fail (menu != null);
+
+        File[] files = null;
+        Gee.List<Granite.Services.Contract> contracts = null;
+
         try {
-            services = service_eactions.GetServicesByLocationsList (build_hash_from_list_selection ());
-        
-            uint i = 0;
-            foreach(var app__ in services)
-            {
-                /* insert separator if we got at least 1 action */
+            if (gof_files == null) {
+                if (current_directory == null)
+                    return;
+
+                files = new File[0];
+                files += current_directory.location;
+
+                string? mimetype = current_directory.get_ftype ();
+
+                if (mimetype == null)
+                    return;
+
+                contracts = Granite.Services.ContractorProxy.get_contracts_by_mime (mimetype);
+            } else {
+                files = get_file_array (gof_files);
+                var mimetypes = get_mimetypes (gof_files);
+                contracts = Granite.Services.ContractorProxy.get_contracts_by_mimelist (mimetypes);
+            }
+
+            assert (files != null);
+            assert (contracts != null);
+
+            for (int i = 0; i < contracts.size; i++) {
+                var contract = contracts.get (i);
+                Gtk.MenuItem menu_item;
+
+                // insert separator if we got at least 1 contract
                 if (i == 0) {
-                    var item = new Gtk.SeparatorMenuItem ();
-                    menu.append (item);
-                    item.show ();
-                    plugins.menus.prepend (item);
+                    menu_item = new Gtk.SeparatorMenuItem ();
+                    add_menuitem (menu, menu_item);
                 }
-                var menuitem = new Gtk.MenuItem.with_label(get_app_display_name(app__));
-                menu.append (menuitem);
-                menuitem.set_data<GLib.HashTable<string,string>> ("app", app__);
-                menuitem.show ();
-                menuitem.activate.connect (action_activated);
-                plugins.menus.prepend (menuitem);
-                i++;
+
+                menu_item = new ContractMenuItem (contract, files);
+                add_menuitem (menu, menu_item);
             }
-        } catch (IOError e) {
-            stderr.printf ("%s\n", e.message);
+        } catch (Error e) {
+            warning (e.message);
         }
     }
 
-    public override void ui (Gtk.UIManager? widget)
-    {
+    public override void ui (Gtk.UIManager? widget) {
         ui_manager = widget;
-        menu = (Gtk.Menu)ui_manager.get_widget("/selection");
-    }
-    
-    public override void directory_loaded (void* user_data)
-    {
-        current_directory = ((Object[])user_data)[2] as GOF.File;
+        menu = (Gtk.Menu) ui_manager.get_widget ("/selection");
+    }
+
+    public override void directory_loaded (void* user_data) {
+        current_directory = ((Object[]) user_data)[2] as GOF.File;
+    }
+
+    private void add_menuitem (Gtk.Menu menu, Gtk.MenuItem menu_item) {
+        menu.append (menu_item);
+        menu_item.show ();
+        plugins.menuitem_references.add (menu_item);
+    }
+
+    private static string[] get_mimetypes (List<GOF.File> files) {
+        string[] mimetypes = new string[files.length ()];
+
+        foreach (var file in files)
+            mimetypes += file.get_ftype () ?? "";
+
+        return mimetypes;
+    }
+
+    private static File[] get_file_array (List<GOF.File> files) {
+        File[] file_array = new File[0];
+
+        foreach (var file in files) {
+            if (file.location != null)
+                file_array += file.location;
+        }
+
+        return file_array;
     }
 }
 
-public Marlin.Plugins.Base module_init ()
-{
+public Marlin.Plugins.Base module_init () {
     return new Marlin.Plugins.Contractor ();
 }
-

=== removed directory 'plugins/extended-actions'
=== removed file 'plugins/extended-actions/CMakeLists.txt'
--- plugins/extended-actions/CMakeLists.txt	2013-04-22 07:45:04 +0000
+++ plugins/extended-actions/CMakeLists.txt	1970-01-01 00:00:00 +0000
@@ -1,28 +0,0 @@
-find_package(PkgConfig)
-pkg_check_modules(DEPS REQUIRED
-	gtk+-3.0
-    gee-1.0
-    glib-2.0)
-set(CFLAGS
-	${DEPS_CFLAGS} ${DEPS_CFLAGS_OTHER}
-)
-include_directories(${CMAKE_BINARY_DIR}/libcore/)
-include_directories(${CMAKE_SOURCE_DIR}/libcore/)
-add_definitions(${CFLAGS})
-link_directories(${LIB_PATHS})
-vala_precompile(VALA_C marlin-extended-actions
-    plugin.vala
-CUSTOM_VAPIS
-    ${CMAKE_BINARY_DIR}/libcore/marlincore.vapi
-    ${CMAKE_SOURCE_DIR}/libcore/marlincore-C.vapi
-    ${CMAKE_BINARY_DIR}/libwidgets/marlinwidgets.vapi
-PACKAGES
-    gtk+-3.0
-    gee-1.0
-OPTIONS
-    --thread)
-add_library(marlin-extended-actions SHARED
-    ${VALA_C})
-target_link_libraries(marlin-extended-actions marlincore marlinwidgets)
-install(TARGETS marlin-extended-actions DESTINATION lib/marlin/plugins/core/)
-install(FILES extended-actions.plug DESTINATION lib/marlin/plugins/core/)

=== removed file 'plugins/extended-actions/extended-actions.plug'
--- plugins/extended-actions/extended-actions.plug	2011-12-04 12:05:59 +0000
+++ plugins/extended-actions/extended-actions.plug	1970-01-01 00:00:00 +0000
@@ -1,3 +0,0 @@
-[Plugin]
-Name=Extended Actions
-File=libmarlin-extended-actions.so

=== removed file 'plugins/extended-actions/plugin.vala'
--- plugins/extended-actions/plugin.vala	2012-06-01 21:34:30 +0000
+++ plugins/extended-actions/plugin.vala	1970-01-01 00:00:00 +0000
@@ -1,156 +0,0 @@
-/*
- * Authors:
- *      Lucas Baudin <[e-mail]>
- *      ammonkey <[e-mail]>
- *      
- * Copyright (C) Lucas Baudin 2011 <[e-mail]>
- * 
- * Marlin is free software: you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- * 
- * Marlin is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- * See the GNU General Public License for more details.
- * 
- * You should have received a copy of the GNU General Public License along
- * with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-using Gtk;
-using Gee;
-
-[DBus (name = "org.magma.ExtendedActions")]
-public interface ExtendedActionsService : Object
-{
-    //public abstract GLib.HashTable<string,string>[] GetServicesByMime (string mime) throws IOError;
-    public abstract GLib.HashTable<string,string>[] GetServicesByLocation (string strlocation, string? file_mime="")    throws IOError;
-    public abstract GLib.HashTable<string,string>[] GetServicesByLocationsList (GLib.HashTable<string, string>[] locations)  throws IOError;
-}
-
-public class Marlin.Plugins.ExtendedActions : Marlin.Plugins.Base
-{
-    UIManager ui_manager;
-    Gtk.Menu menu;
-    GOF.File current_directory = null;
-    unowned GLib.List<GOF.File> selection;
-    GLib.HashTable<string,string>[] services = null;
-    
-    private ExtendedActionsService service_eactions;
-    
-    public ExtendedActions ()
-    {
-        try {
-            service_eactions = Bus.get_proxy_sync (BusType.SESSION,
-                                                   "org.magma.ExtendedActions",
-                                                   "/org/magma/ExtendedActions");
-        } catch (IOError e) {
-            stderr.printf ("%s\n", e.message);
-        }
-    }
-
-    private string get_app_display_name (GLib.HashTable<string,string> app__)
-    {
-        return app__.lookup ("Description");
-    }
-
-    private GLib.HashTable<string,string> add_location_entry (GOF.File file)
-    {
-        GLib.HashTable<string,string> entry;
-                
-        entry = new GLib.HashTable<string,string> (str_hash, str_equal);
-        entry.insert ("uri", file.uri);
-        var ftype = file.get_ftype ();
-        if (ftype == "application/octet-stream" || ftype == null)
-            entry.insert ("mimetype", "");
-        else
-            entry.insert ("mimetype", ftype);
-
-        return entry;
-    }
-
-    private GLib.HashTable<string, string>[] build_hash_from_list_selection ()
-    {
-        GLib.HashTable<string,string>[] locations = null;
-
-        foreach (GOF.File file in selection) {
-            if (file != null)
-                locations += add_location_entry (file);
-            //message ("file %s", file.name);
-        }
-        if (selection == null && current_directory != null) {
-            locations += add_location_entry (current_directory);
-        }
-
-        return locations;
-    }
-
-    public void action_activated ()
-    {
-        Gtk.MenuItem menuitem;
-        GLib.HashTable<string,string> app__;
-                
-        menuitem = (Gtk.MenuItem) menu.get_active ();
-        app__ = menuitem.get_data<GLib.HashTable<string,string>> ("app");
-
-        if (app__ != null) {
-            var cmd = app__.lookup ("Exec");
-            //message ("test exec %s", cmd);
-            try {
-                GLib.Process.spawn_command_line_async (cmd);
-            } catch (SpawnError e) {
-                stderr.printf ("error spawn command line %s: %s", cmd, e.message);
-            }
-        }
-    }
-
-    public override void context_menu (Gtk.Widget? widget, GLib.List<GOF.File> files)
-    {
-        menu = widget as Gtk.Menu;
-        
-        selection = files;
-        try {
-            services = service_eactions.GetServicesByLocationsList (build_hash_from_list_selection ());
-        
-            uint i = 0;
-            foreach(var app__ in services)
-            {
-                /* insert separator if we got at least 1 action */
-                if (i == 0) {
-                    var item = new Gtk.SeparatorMenuItem ();
-                    menu.append (item);
-                    item.show ();
-                    plugins.menus.prepend (item);
-                }
-                var menuitem = new Gtk.MenuItem.with_label(get_app_display_name(app__));
-                menu.append (menuitem);
-                menuitem.set_data<GLib.HashTable<string,string>> ("app", app__);
-                menuitem.show ();
-                menuitem.activate.connect (action_activated);
-                plugins.menus.prepend (menuitem);
-                i++;
-            }
-        } catch (IOError e) {
-            stderr.printf ("%s\n", e.message);
-        }
-    }
-
-    public override void ui (Gtk.UIManager? widget)
-    {
-        ui_manager = widget;
-        menu = (Gtk.Menu)ui_manager.get_widget("/selection");
-    }
-    
-    public override void directory_loaded (void* user_data)
-    {
-        current_directory = ((Object[])user_data)[2] as GOF.File;
-    }
-}
-
-public Marlin.Plugins.Base module_init ()
-{
-    return new Marlin.Plugins.ExtendedActions ();
-}
-