Merge lp:~ivanfateev/pantheon-files/fix-1380139 into lp:~elementary-apps/pantheon-files/trunk

Proposed by Ivan Fateev
Status: Rejected
Rejected by: Jeremy Wootten
Proposed branch: lp:~ivanfateev/pantheon-files/fix-1380139
Merge into: lp:~elementary-apps/pantheon-files/trunk
Diff against target: 153 lines (+70/-20)
2 files modified
src/CMakeLists.txt (+2/-0)
src/View/AbstractDirectoryView.vala (+68/-20)
To merge this branch: bzr merge lp:~ivanfateev/pantheon-files/fix-1380139
Reviewer Review Type Date Requested Status
Jeremy Wootten Disapprove
Review via email: mp+257797@code.launchpad.net

Description of the change

Added support for gnome-sushi previewer according to this bug
https://bugs.launchpad.net/pantheon-files/+bug/1380139

How to test:
- install gnome-sushi. Select file, hit space, previewer should open
- remove gnome-sushi. Select file, hit space, pantheon-files should behave as was designed before, open select file

To post a comment you must log in.
1814. By Ivan Fateev

- added full support for previewer gnome-sushi
- added enum PreviewerType - maybe files will support more previewers
- if gnome-sushi is not installed - behaviour is default (open selected file)

1815. By Ivan Fateev

removed debug messages about previewer

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

The problem with this is that installing gnome-sushi also installs Nautilus, which does not seem desirable. Is it possible to remove this dependency on Nautilus?

Revision history for this message
Jeremy Wootten (jeremywootten) :
review: Needs Fixing
Revision history for this message
Ivan Fateev (ivanfateev) wrote :

gnome-sushi works fine without nautilus. How we can get rid of this dependency?
Unfortunately I don't have experience with packaging. Should we file a bug? If so then for which version?

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

I'll ask Cody. Possibly we could build our own gnome-sushi package which doesn't depend on Nautilus, instead of the Ubuntu one and have the pantheon-files package recommend it.

I'll continue the review on the assumption that this issue is not a merge blocker.

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

After discussion with some of the other developers, it has been decided to reject this branch for the following reasons:

1) It would bring in unwanted dependencies e.g. on gjs and X11
2) It is an elementary design decision not support a previewer in Files; there is little difference in time for the file to open in the default app compared to the previewer and the intention is to reduce any time difference further.

Thank you for your work; I am sorry we cannot accept it.

review: Disapprove

Unmerged revisions

1815. By Ivan Fateev

removed debug messages about previewer

1814. By Ivan Fateev

- added full support for previewer gnome-sushi
- added enum PreviewerType - maybe files will support more previewers
- if gnome-sushi is not installed - behaviour is default (open selected file)

1813. By Ivan Fateev

integrated gnome-sushi previewer via dbus. TODO: check whether gnome-sushi is installed. Added gdk-x11-3.0 as dependency

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/CMakeLists.txt'
2--- src/CMakeLists.txt 2015-01-18 20:46:03 +0000
3+++ src/CMakeLists.txt 2015-04-30 17:08:23 +0000
4@@ -20,6 +20,7 @@
5 gio-unix-2.0
6 pango>=1.1.2
7 gtk+-3.0>=3.10
8+ gdk-x11-3.0
9 gmodule-2.0
10 gail-3.0
11 gee-0.8
12@@ -87,6 +88,7 @@
13 View/Chrome/ImgEventBox.vala
14 PACKAGES
15 gtk+-3.0
16+ gdk-x11-3.0
17 gio-2.0
18 posix
19 gee-0.8
20
21=== modified file 'src/View/AbstractDirectoryView.vala'
22--- src/View/AbstractDirectoryView.vala 2015-04-18 12:10:52 +0000
23+++ src/View/AbstractDirectoryView.vala 2015-04-30 17:08:23 +0000
24@@ -25,6 +25,11 @@
25 namespace FM {
26 public abstract class AbstractDirectoryView : Gtk.ScrolledWindow {
27
28+ public enum PreviewerType {
29+ NONE,
30+ GNOME_SUSHI
31+ }
32+
33 public enum TargetType {
34 STRING,
35 TEXT_URI_LIST,
36@@ -44,6 +49,12 @@
37
38 const int MAX_TEMPLATES = 32;
39
40+ const string GNOME_SUSHI_DBUS_NAME = "org.gnome.NautilusPreviewer";
41+ const string GNOME_SUSHI_DBUS_IFACE = "org.gnome.NautilusPreviewer";
42+ const string GNOME_SUSHI_DBUS_PATH = "/org/gnome/NautilusPreviewer";
43+ const string GNOME_SUSHI_PACKAGE_NAME = "gnome-sushi";
44+ const string DPKG_COMMAND_FORMAT = "dpkg --get-selections %s";
45+
46 const Gtk.TargetEntry [] drag_targets = {
47 {"text/plain", 0, TargetType.STRING},
48 {"text/uri-list", 0, TargetType.TEXT_URI_LIST}
49@@ -164,7 +175,7 @@
50 Marlin.Thumbnailer thumbnailer = null;
51
52 /* TODO Support for preview see bug #1380139 */
53- private string? previewer = null;
54+ private PreviewerType previewer_type = PreviewerType.NONE;
55
56 /* Rename support */
57 protected Marlin.TextRenderer? name_renderer = null;
58@@ -285,6 +296,7 @@
59 freeze_tree (); /* speed up loading of icon view. Thawed when directory loaded */
60 set_up_zoom_level ();
61 change_zoom_level ();
62+ set_up_previewer.begin ();
63 }
64
65 ~AbstractDirectoryView () {
66@@ -532,25 +544,40 @@
67 }
68
69 protected void preview_selected_items () {
70- if (previewer == null) /* At present this is the case! */
71- activate_selected_items (Marlin.OpenFlag.DEFAULT);
72- else {
73- unowned GLib.List<unowned GOF.File>? selection = get_selected_files ();
74-
75- if (selection == null)
76- return;
77-
78- Gdk.Screen screen = Eel.gtk_widget_get_screen (this);
79- GLib.List<GLib.File> location_list = null;
80- GOF.File file = selection.data;
81- location_list.prepend (file.location);
82- Gdk.AppLaunchContext context = screen.get_display ().get_app_launch_context ();
83- try {
84- GLib.AppInfo previewer_app = GLib.AppInfo.create_from_commandline (previewer, null, 0);
85- previewer_app.launch (location_list, context as GLib.AppLaunchContext);
86- } catch (GLib.Error error) {
87- Eel.show_error_dialog (_("Failed to preview"), error.message, null);
88- }
89+ unowned GLib.List<unowned GOF.File>? selection = get_selected_files ();
90+
91+ if (selection == null)
92+ return;
93+
94+ switch (previewer_type) {
95+ case PreviewerType.NONE:
96+ activate_selected_items (Marlin.OpenFlag.DEFAULT);
97+ break;
98+ case PreviewerType.GNOME_SUSHI:
99+ var dbus = window.application.get_dbus_connection ();
100+ GOF.File file = selection.data;
101+ var xid = ((Gdk.X11.Window)get_window ()).get_xid ();
102+ var params = new GLib.Variant ("(sib)", file.uri, xid, true);
103+ dbus.call.begin (
104+ GNOME_SUSHI_DBUS_NAME,
105+ GNOME_SUSHI_DBUS_PATH,
106+ GNOME_SUSHI_DBUS_IFACE,
107+ "ShowFile",
108+ params,
109+ null,
110+ GLib.DBusCallFlags.NONE,
111+ -1,
112+ null,
113+ (obj, res) => {
114+ try {
115+ dbus.call.end(res);
116+ } catch (GLib.Error error) {
117+ Eel.show_error_dialog (_("Failed to preview"), error.message, null);
118+ }
119+ }
120+ );
121+ break;
122+
123 }
124 }
125
126@@ -3045,6 +3072,27 @@
127 }
128 }
129
130+ /** Previewer **/
131+ private async void set_up_previewer () {
132+ string cmd_stdout;
133+ string cmd_stderr;
134+ int cmd_status;
135+ try {
136+ Process.spawn_command_line_sync (
137+ DPKG_COMMAND_FORMAT.printf (GNOME_SUSHI_PACKAGE_NAME),
138+ out cmd_stdout,
139+ out cmd_stderr,
140+ out cmd_status
141+ );
142+
143+ if (cmd_stdout.index_of ("install") > 0) {
144+ previewer_type = PreviewerType.GNOME_SUSHI;
145+ }
146+ } catch (SpawnError e) {
147+ error ("Failed to launch dpkg");
148+ }
149+ }
150+
151 public virtual void sync_selection () {}
152 public virtual void highlight_path (Gtk.TreePath? path) {}
153 protected virtual void add_subdirectory (GOF.Directory.Async dir) {}

Subscribers

People subscribed via source and target branches

to all changes: