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

Proposed by Victor Martinez on 2013-04-23
Status: Merged
Merged at revision: 1157
Proposed branch: lp:~victored/pantheon-files/contractor-plugin
Merge into: lp:~elementary-apps/pantheon-files/trunk
Diff against target: 555 lines (+143/-311)
7 files modified
libcore/PluginManager.vala (+34/-2)
plugins/CMakeLists.txt (+0/-1)
plugins/contractor/CMakeLists.txt (+3/-1)
plugins/contractor/plugin.vala (+106/-120)
plugins/extended-actions/CMakeLists.txt (+0/-28)
plugins/extended-actions/extended-actions.plug (+0/-3)
plugins/extended-actions/plugin.vala (+0/-156)
To merge this branch: bzr merge lp:~victored/pantheon-files/contractor-plugin
Reviewer Review Type Date Requested Status
Sergey "Shnatsel" Davidoff (community) 2013-04-23 Approve on 2013-04-24
Review via email: mp+160377@code.launchpad.net

Description of the change

- 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.

To post a comment you must log in.
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.

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

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

review: Needs Fixing

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

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

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) wrote :

Hey Sergey, thank you for testing!

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

Works properly for me too.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libcore/PluginManager.vala'
2--- libcore/PluginManager.vala 2012-11-06 19:52:04 +0000
3+++ libcore/PluginManager.vala 2013-04-24 17:03:26 +0000
4@@ -30,7 +30,11 @@
5 string plugin_dir;
6 Gee.List<string> names;
7 bool in_available = false;
8- public GLib.List<Gtk.Widget>? menus;
9+
10+ [Deprecated (replacement = "Marlin.PluginManager.menuitem_references")]
11+ public GLib.List<Gtk.Widget>? menus; // this doesn't manage GObject references properly
12+
13+ public Gee.List<Gtk.Widget> menuitem_references { get; private set; }
14
15 public PluginManager(Settings settings, string field, string plugin_dir)
16 {
17@@ -39,6 +43,8 @@
18 this.plugin_dir = plugin_dir;
19 plugin_hash = new Gee.HashMap<string,Plugins.Base>();
20 names = new Gee.ArrayList<string>();
21+
22+ menuitem_references = new Gee.LinkedList<Gtk.Widget> ();
23 }
24
25 public void load_plugins()
26@@ -147,10 +153,36 @@
27
28 public void hook_context_menu(Gtk.Widget menu, List<GOF.File> files)
29 {
30+ drop_menu_references (menu);
31+
32+ if (menu is Gtk.Menu)
33+ drop_plugin_menuitems (menu as Gtk.Menu);
34+
35+
36+ foreach (var plugin in plugin_hash.values)
37+ plugin.context_menu (menu, files);
38+ }
39+
40+ private void drop_plugin_menuitems (Gtk.Menu menu) {
41+ var plugin_menu = menu as Gtk.Menu;
42+
43+ assert (plugin_menu != null);
44+
45+ foreach (var menu_item in menuitem_references)
46+ menu_item.parent.remove (menu_item);
47+
48+ menuitem_references.clear ();
49+ }
50+
51+ [Deprecated (replacement = "Marlin.PluginManager.drop_plugin_menuitems")]
52+ private void drop_menu_references (Gtk.Widget menu) {
53+ if (menus == null)
54+ return;
55+
56 foreach (var item in menus)
57 item.destroy ();
58+
59 menus = null;
60- foreach(var plugin in plugin_hash.values) plugin.context_menu (menu, files);
61 }
62
63 public void ui(Gtk.UIManager data)
64
65=== modified file 'plugins/CMakeLists.txt'
66--- plugins/CMakeLists.txt 2012-06-01 21:34:30 +0000
67+++ plugins/CMakeLists.txt 2013-04-24 17:03:26 +0000
68@@ -1,4 +1,3 @@
69-add_subdirectory(extended-actions)
70 add_subdirectory(contractor)
71 add_subdirectory(marlin-trash)
72 add_subdirectory(marlin-ctags)
73
74=== modified file 'plugins/contractor/CMakeLists.txt'
75--- plugins/contractor/CMakeLists.txt 2013-04-22 07:45:04 +0000
76+++ plugins/contractor/CMakeLists.txt 2013-04-24 17:03:26 +0000
77@@ -2,7 +2,8 @@
78 pkg_check_modules(DEPS REQUIRED
79 gtk+-3.0
80 gee-1.0
81- glib-2.0)
82+ glib-2.0
83+ granite)
84 set(CFLAGS
85 ${DEPS_CFLAGS} ${DEPS_CFLAGS_OTHER}
86 )
87@@ -19,6 +20,7 @@
88 PACKAGES
89 gtk+-3.0
90 gee-1.0
91+ granite
92 OPTIONS
93 --thread)
94 add_library(marlin-contractor SHARED
95
96=== modified file 'plugins/contractor/plugin.vala'
97--- plugins/contractor/plugin.vala 2012-06-02 15:49:15 +0000
98+++ plugins/contractor/plugin.vala 2013-04-24 17:03:26 +0000
99@@ -2,8 +2,10 @@
100 * Authors:
101 * Lucas Baudin <xapantu@gmail.com>
102 * ammonkey <am.monkeyd@gmail.com>
103- *
104+ * Victor Martinez <victoreduardm@gmail.com>
105+ *
106 * Copyright (C) Lucas Baudin 2011 <xapantu@gmail.com>
107+ * Copyright (C) 2013 elementary
108 *
109 * Marlin is free software: you can redistribute it and/or modify it
110 * under the terms of the GNU General Public License as published by the
111@@ -19,138 +21,122 @@
112 * with this program. If not, see <http://www.gnu.org/licenses/>.
113 */
114
115-using Gtk;
116-using Gee;
117-
118-[DBus (name = "org.elementary.contractor")]
119-public interface ContractorService : Object
120-{
121- //public abstract GLib.HashTable<string,string>[] GetServicesByMime (string mime) throws IOError;
122- public abstract GLib.HashTable<string,string>[] GetServicesByLocation (string strlocation, string? file_mime="") throws IOError;
123- public abstract GLib.HashTable<string,string>[] GetServicesByLocationsList (GLib.HashTable<string, string>[] locations) throws IOError;
124+public class Marlin.Plugins.ContractMenuItem : Gtk.MenuItem {
125+ private Granite.Services.Contract contract;
126+ private File[] files;
127+
128+ public ContractMenuItem (Granite.Services.Contract contract, File[] files) {
129+ this.contract = contract;
130+ this.files = files;
131+
132+ label = contract.get_display_name ();
133+ tooltip_text = contract.get_description ();
134+ }
135+
136+ public override void activate () {
137+ try {
138+ if (files.length == 1)
139+ contract.execute_with_file (files[0]);
140+ else
141+ contract.execute_with_files (files);
142+ } catch (Error err) {
143+ warning (err.message);
144+ }
145+ }
146 }
147
148-public class Marlin.Plugins.Contractor : Marlin.Plugins.Base
149-{
150- UIManager ui_manager;
151- Gtk.Menu menu;
152- GOF.File current_directory = null;
153- unowned GLib.List<GOF.File> selection;
154- GLib.HashTable<string,string>[] services = null;
155-
156- private ContractorService service_eactions;
157-
158- public Contractor ()
159- {
160- try {
161- service_eactions = Bus.get_proxy_sync (BusType.SESSION,
162- "org.elementary.contractor",
163- "/org/elementary/contractor");
164- } catch (IOError e) {
165- stderr.printf ("%s\n", e.message);
166- }
167- }
168-
169- private string get_app_display_name (GLib.HashTable<string,string> app__)
170- {
171- return app__.lookup ("Description");
172- }
173-
174- private GLib.HashTable<string,string> add_location_entry (GOF.File file)
175- {
176- GLib.HashTable<string,string> entry;
177-
178- entry = new GLib.HashTable<string,string> (str_hash, str_equal);
179- entry.insert ("uri", file.uri);
180- var ftype = file.get_ftype ();
181- if (ftype == "application/octet-stream" || ftype == null)
182- entry.insert ("mimetype", "");
183- else
184- entry.insert ("mimetype", ftype);
185-
186- return entry;
187- }
188-
189- private GLib.HashTable<string, string>[] build_hash_from_list_selection ()
190- {
191- GLib.HashTable<string,string>[] locations = null;
192-
193- foreach (GOF.File file in selection) {
194- if (file != null)
195- locations += add_location_entry (file);
196- //message ("file %s", file.name);
197- }
198- if (selection == null && current_directory != null) {
199- locations += add_location_entry (current_directory);
200- }
201-
202- return locations;
203- }
204-
205- public void action_activated ()
206- {
207- Gtk.MenuItem menuitem;
208- GLib.HashTable<string,string> app__;
209-
210- menuitem = (Gtk.MenuItem) menu.get_active ();
211- app__ = menuitem.get_data<GLib.HashTable<string,string>> ("app");
212-
213- if (app__ != null) {
214- var cmd = app__.lookup ("Exec");
215- //message ("test exec %s", cmd);
216- try {
217- GLib.Process.spawn_command_line_async (cmd);
218- } catch (SpawnError e) {
219- stderr.printf ("error spawn command line %s: %s", cmd, e.message);
220- }
221- }
222- }
223-
224- public override void context_menu (Gtk.Widget? widget, GLib.List<GOF.File> files)
225- {
226+public class Marlin.Plugins.Contractor : Marlin.Plugins.Base {
227+ private Gtk.UIManager ui_manager;
228+ private Gtk.Menu menu;
229+ private GOF.File current_directory = null;
230+
231+ public Contractor () {
232+ }
233+
234+ public override void context_menu (Gtk.Widget? widget, List<GOF.File> gof_files) {
235 menu = widget as Gtk.Menu;
236-
237- selection = files;
238+ return_if_fail (menu != null);
239+
240+ File[] files = null;
241+ Gee.List<Granite.Services.Contract> contracts = null;
242+
243 try {
244- services = service_eactions.GetServicesByLocationsList (build_hash_from_list_selection ());
245-
246- uint i = 0;
247- foreach(var app__ in services)
248- {
249- /* insert separator if we got at least 1 action */
250+ if (gof_files == null) {
251+ if (current_directory == null)
252+ return;
253+
254+ files = new File[0];
255+ files += current_directory.location;
256+
257+ string? mimetype = current_directory.get_ftype ();
258+
259+ if (mimetype == null)
260+ return;
261+
262+ contracts = Granite.Services.ContractorProxy.get_contracts_by_mime (mimetype);
263+ } else {
264+ files = get_file_array (gof_files);
265+ var mimetypes = get_mimetypes (gof_files);
266+ contracts = Granite.Services.ContractorProxy.get_contracts_by_mimelist (mimetypes);
267+ }
268+
269+ assert (files != null);
270+ assert (contracts != null);
271+
272+ for (int i = 0; i < contracts.size; i++) {
273+ var contract = contracts.get (i);
274+ Gtk.MenuItem menu_item;
275+
276+ // insert separator if we got at least 1 contract
277 if (i == 0) {
278- var item = new Gtk.SeparatorMenuItem ();
279- menu.append (item);
280- item.show ();
281- plugins.menus.prepend (item);
282+ menu_item = new Gtk.SeparatorMenuItem ();
283+ add_menuitem (menu, menu_item);
284 }
285- var menuitem = new Gtk.MenuItem.with_label(get_app_display_name(app__));
286- menu.append (menuitem);
287- menuitem.set_data<GLib.HashTable<string,string>> ("app", app__);
288- menuitem.show ();
289- menuitem.activate.connect (action_activated);
290- plugins.menus.prepend (menuitem);
291- i++;
292+
293+ menu_item = new ContractMenuItem (contract, files);
294+ add_menuitem (menu, menu_item);
295 }
296- } catch (IOError e) {
297- stderr.printf ("%s\n", e.message);
298+ } catch (Error e) {
299+ warning (e.message);
300 }
301 }
302
303- public override void ui (Gtk.UIManager? widget)
304- {
305+ public override void ui (Gtk.UIManager? widget) {
306 ui_manager = widget;
307- menu = (Gtk.Menu)ui_manager.get_widget("/selection");
308- }
309-
310- public override void directory_loaded (void* user_data)
311- {
312- current_directory = ((Object[])user_data)[2] as GOF.File;
313+ menu = (Gtk.Menu) ui_manager.get_widget ("/selection");
314+ }
315+
316+ public override void directory_loaded (void* user_data) {
317+ current_directory = ((Object[]) user_data)[2] as GOF.File;
318+ }
319+
320+ private void add_menuitem (Gtk.Menu menu, Gtk.MenuItem menu_item) {
321+ menu.append (menu_item);
322+ menu_item.show ();
323+ plugins.menuitem_references.add (menu_item);
324+ }
325+
326+ private static string[] get_mimetypes (List<GOF.File> files) {
327+ string[] mimetypes = new string[files.length ()];
328+
329+ foreach (var file in files)
330+ mimetypes += file.get_ftype () ?? "";
331+
332+ return mimetypes;
333+ }
334+
335+ private static File[] get_file_array (List<GOF.File> files) {
336+ File[] file_array = new File[0];
337+
338+ foreach (var file in files) {
339+ if (file.location != null)
340+ file_array += file.location;
341+ }
342+
343+ return file_array;
344 }
345 }
346
347-public Marlin.Plugins.Base module_init ()
348-{
349+public Marlin.Plugins.Base module_init () {
350 return new Marlin.Plugins.Contractor ();
351 }
352-
353
354=== removed directory 'plugins/extended-actions'
355=== removed file 'plugins/extended-actions/CMakeLists.txt'
356--- plugins/extended-actions/CMakeLists.txt 2013-04-22 07:45:04 +0000
357+++ plugins/extended-actions/CMakeLists.txt 1970-01-01 00:00:00 +0000
358@@ -1,28 +0,0 @@
359-find_package(PkgConfig)
360-pkg_check_modules(DEPS REQUIRED
361- gtk+-3.0
362- gee-1.0
363- glib-2.0)
364-set(CFLAGS
365- ${DEPS_CFLAGS} ${DEPS_CFLAGS_OTHER}
366-)
367-include_directories(${CMAKE_BINARY_DIR}/libcore/)
368-include_directories(${CMAKE_SOURCE_DIR}/libcore/)
369-add_definitions(${CFLAGS})
370-link_directories(${LIB_PATHS})
371-vala_precompile(VALA_C marlin-extended-actions
372- plugin.vala
373-CUSTOM_VAPIS
374- ${CMAKE_BINARY_DIR}/libcore/marlincore.vapi
375- ${CMAKE_SOURCE_DIR}/libcore/marlincore-C.vapi
376- ${CMAKE_BINARY_DIR}/libwidgets/marlinwidgets.vapi
377-PACKAGES
378- gtk+-3.0
379- gee-1.0
380-OPTIONS
381- --thread)
382-add_library(marlin-extended-actions SHARED
383- ${VALA_C})
384-target_link_libraries(marlin-extended-actions marlincore marlinwidgets)
385-install(TARGETS marlin-extended-actions DESTINATION lib/marlin/plugins/core/)
386-install(FILES extended-actions.plug DESTINATION lib/marlin/plugins/core/)
387
388=== removed file 'plugins/extended-actions/extended-actions.plug'
389--- plugins/extended-actions/extended-actions.plug 2011-12-04 12:05:59 +0000
390+++ plugins/extended-actions/extended-actions.plug 1970-01-01 00:00:00 +0000
391@@ -1,3 +0,0 @@
392-[Plugin]
393-Name=Extended Actions
394-File=libmarlin-extended-actions.so
395
396=== removed file 'plugins/extended-actions/plugin.vala'
397--- plugins/extended-actions/plugin.vala 2012-06-01 21:34:30 +0000
398+++ plugins/extended-actions/plugin.vala 1970-01-01 00:00:00 +0000
399@@ -1,156 +0,0 @@
400-/*
401- * Authors:
402- * Lucas Baudin <xapantu@gmail.com>
403- * ammonkey <am.monkeyd@gmail.com>
404- *
405- * Copyright (C) Lucas Baudin 2011 <xapantu@gmail.com>
406- *
407- * Marlin is free software: you can redistribute it and/or modify it
408- * under the terms of the GNU General Public License as published by the
409- * Free Software Foundation, either version 3 of the License, or
410- * (at your option) any later version.
411- *
412- * Marlin is distributed in the hope that it will be useful, but
413- * WITHOUT ANY WARRANTY; without even the implied warranty of
414- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
415- * See the GNU General Public License for more details.
416- *
417- * You should have received a copy of the GNU General Public License along
418- * with this program. If not, see <http://www.gnu.org/licenses/>.
419- */
420-
421-using Gtk;
422-using Gee;
423-
424-[DBus (name = "org.magma.ExtendedActions")]
425-public interface ExtendedActionsService : Object
426-{
427- //public abstract GLib.HashTable<string,string>[] GetServicesByMime (string mime) throws IOError;
428- public abstract GLib.HashTable<string,string>[] GetServicesByLocation (string strlocation, string? file_mime="") throws IOError;
429- public abstract GLib.HashTable<string,string>[] GetServicesByLocationsList (GLib.HashTable<string, string>[] locations) throws IOError;
430-}
431-
432-public class Marlin.Plugins.ExtendedActions : Marlin.Plugins.Base
433-{
434- UIManager ui_manager;
435- Gtk.Menu menu;
436- GOF.File current_directory = null;
437- unowned GLib.List<GOF.File> selection;
438- GLib.HashTable<string,string>[] services = null;
439-
440- private ExtendedActionsService service_eactions;
441-
442- public ExtendedActions ()
443- {
444- try {
445- service_eactions = Bus.get_proxy_sync (BusType.SESSION,
446- "org.magma.ExtendedActions",
447- "/org/magma/ExtendedActions");
448- } catch (IOError e) {
449- stderr.printf ("%s\n", e.message);
450- }
451- }
452-
453- private string get_app_display_name (GLib.HashTable<string,string> app__)
454- {
455- return app__.lookup ("Description");
456- }
457-
458- private GLib.HashTable<string,string> add_location_entry (GOF.File file)
459- {
460- GLib.HashTable<string,string> entry;
461-
462- entry = new GLib.HashTable<string,string> (str_hash, str_equal);
463- entry.insert ("uri", file.uri);
464- var ftype = file.get_ftype ();
465- if (ftype == "application/octet-stream" || ftype == null)
466- entry.insert ("mimetype", "");
467- else
468- entry.insert ("mimetype", ftype);
469-
470- return entry;
471- }
472-
473- private GLib.HashTable<string, string>[] build_hash_from_list_selection ()
474- {
475- GLib.HashTable<string,string>[] locations = null;
476-
477- foreach (GOF.File file in selection) {
478- if (file != null)
479- locations += add_location_entry (file);
480- //message ("file %s", file.name);
481- }
482- if (selection == null && current_directory != null) {
483- locations += add_location_entry (current_directory);
484- }
485-
486- return locations;
487- }
488-
489- public void action_activated ()
490- {
491- Gtk.MenuItem menuitem;
492- GLib.HashTable<string,string> app__;
493-
494- menuitem = (Gtk.MenuItem) menu.get_active ();
495- app__ = menuitem.get_data<GLib.HashTable<string,string>> ("app");
496-
497- if (app__ != null) {
498- var cmd = app__.lookup ("Exec");
499- //message ("test exec %s", cmd);
500- try {
501- GLib.Process.spawn_command_line_async (cmd);
502- } catch (SpawnError e) {
503- stderr.printf ("error spawn command line %s: %s", cmd, e.message);
504- }
505- }
506- }
507-
508- public override void context_menu (Gtk.Widget? widget, GLib.List<GOF.File> files)
509- {
510- menu = widget as Gtk.Menu;
511-
512- selection = files;
513- try {
514- services = service_eactions.GetServicesByLocationsList (build_hash_from_list_selection ());
515-
516- uint i = 0;
517- foreach(var app__ in services)
518- {
519- /* insert separator if we got at least 1 action */
520- if (i == 0) {
521- var item = new Gtk.SeparatorMenuItem ();
522- menu.append (item);
523- item.show ();
524- plugins.menus.prepend (item);
525- }
526- var menuitem = new Gtk.MenuItem.with_label(get_app_display_name(app__));
527- menu.append (menuitem);
528- menuitem.set_data<GLib.HashTable<string,string>> ("app", app__);
529- menuitem.show ();
530- menuitem.activate.connect (action_activated);
531- plugins.menus.prepend (menuitem);
532- i++;
533- }
534- } catch (IOError e) {
535- stderr.printf ("%s\n", e.message);
536- }
537- }
538-
539- public override void ui (Gtk.UIManager? widget)
540- {
541- ui_manager = widget;
542- menu = (Gtk.Menu)ui_manager.get_widget("/selection");
543- }
544-
545- public override void directory_loaded (void* user_data)
546- {
547- current_directory = ((Object[])user_data)[2] as GOF.File;
548- }
549-}
550-
551-public Marlin.Plugins.Base module_init ()
552-{
553- return new Marlin.Plugins.ExtendedActions ();
554-}
555-

Subscribers

People subscribed via source and target branches

to all changes: