Merge lp:~jeremywootten/pantheon-files/fix-1669996-execute-edit-script-choice into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Confirm execution of script or other executable when clicked on.

This branch shows a dialog if an executable script (or x-application) is clicked on, asking to confirm execution of the file and, if a default application is available, giving the option to open it with the default application instead.

The dialog does not appear if "Run" is deliberately chosen from the context menu.

[David Hewitt (davidmhewitt)]

Code looks good, haven't tested yet. One minor comment about the wording. Does it make more sense to say "This file may be run as a program..." or maybe even "application" instead of "program" instead of "This file may be executed as a program"?

I feel as though run would be more widely understood than execute.

[Jeremy Wootten (jeremywootten)]

The wording is provisional and needs UX team approval really. I do not think "application" is necessarily correct - the file is likely to be a script which is not an application in the normal sense. Indeed, strictly speaking, it is not definitely a program is it? - it is a file with the attribute of being executable. I used the plainer word "Run" in the main message and the more technical word "execute" in the detailed message, but I am open to using the same word everywhere.

[Danielle Foré (danrabbit)]

I'm kind of -1 on this branch. Throwing a dialog doesn't make it faster for people who want to run the script. It's still two clicks, but now there's an annoying dialog in the way whenever I want to edit a script. And I'm a bit of the position that I'm going to be executing scripts from Terminal anyways because I want the output.

review: Disapprove

Unmerged revisions

2522. By Jeremy Wootten on 2017-03-11

Implement ConfirmExecuteDialog

=== modified file 'libcore/gof-file.c'
--- libcore/gof-file.c	2017-02-26 04:50:36 +0000
+++ libcore/gof-file.c	2017-03-11 18:39:16 +0000
@@ -1477,7 +1477,7 @@
             /* check for .exe, .bar or .com */
             can_execute = g_content_type_can_be_executable (content_type);
 #else
-            /* check if the content type is save to execute, we don't use
+            /* check if the content type is safe to execute, we don't use
              * g_content_type_can_be_executable() for unix because it also returns
              * true for "text/plain" and we don't want that */
             if (g_content_type_is_a (content_type, "application/x-executable")

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2017-02-01 14:42:37 +0000
+++ src/CMakeLists.txt	2017-03-11 18:39:16 +0000
@@ -81,6 +81,7 @@
         Thumbnailer.vala
         Dialogs/AbstractPropertiesDialog.vala
         Dialogs/ChooseAppDialog.vala
+        Dialogs/ConfirmExecuteDialog.vala
         Dialogs/PropertiesWindow.vala
         Dialogs/VolumePropertiesWindow.vala
         Utils/MimeActions.vala
@@ -147,6 +148,7 @@
         Thumbnailer.vala
         Dialogs/AbstractPropertiesDialog.vala
         Dialogs/ChooseAppDialog.vala
+        Dialogs/ConfirmExecuteDialog.vala
         Dialogs/PropertiesWindow.vala
         Dialogs/VolumePropertiesWindow.vala
         Utils/MimeActions.vala

=== added file 'src/Dialogs/ConfirmExecuteDialog.vala'
--- src/Dialogs/ConfirmExecuteDialog.vala	1970-01-01 00:00:00 +0000
+++ src/Dialogs/ConfirmExecuteDialog.vala	2017-03-11 18:39:16 +0000
@@ -0,0 +1,79 @@
+/*
+* Copyright (c) 2017 elementary LLC (http://launchpad.net/pantheon-files)
+*
+* This program 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, Inc.,; either
+* version 3 of the License, or (at your option) any later version.
+*
+* This program 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, write to the
+* Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+* Boston, MA 02110-1335 USA.
+*
+* Authored by: Jeremy Wootten <[e-mail]>
+*/
+
+public class PF.ConfirmExecuteDialog : Object {
+    public enum Response {
+        CANCEL,
+        EXECUTE,
+        OPEN
+    }
+
+    private Gtk.MessageDialog dialog;
+    public GOF.File file {get; construct;}
+    public GLib.AppInfo? default_app {get; construct;}
+    public Gtk.Window? parent {get; construct;}
+
+    public ConfirmExecuteDialog (GOF.File _file, GLib.AppInfo? _default_app, Gtk.Window? _parent) {
+        Object (file: _file,
+                default_app: _default_app,
+                parent: _parent
+        );
+    }
+
+    construct {
+        string primary_text, secondary_text;
+
+        if (default_app != null) {
+           primary_text =  _("Run or Open this File?").printf (file.basename);
+            secondary_text = _("This file may be executed as a program or opened with the default application (%s).\n\nOnly execute files that you know to be safe").printf (default_app.get_name ());
+        } else {
+           primary_text =  _("Run this File?").printf (file.basename);
+           secondary_text = _("This file may be executed as a program.\n\nOnly execute files that you know to be safe").printf (default_app.get_name ());
+        }
+
+        dialog = new Gtk.MessageDialog (parent,
+                                        Gtk.DialogFlags.MODAL | Gtk.DialogFlags.DESTROY_WITH_PARENT,
+                                        Gtk.MessageType.QUESTION,
+                                        Gtk.ButtonsType.NONE,
+                                        primary_text);
+
+        dialog.secondary_text = secondary_text;
+
+        dialog.add_button (_("Execute"), Response.EXECUTE);
+        dialog.add_button (_("Cancel"), Response.CANCEL);
+
+        /* Set safer option to be the default */
+        if (default_app != null) {
+            dialog.add_button (_("Open"), Response.OPEN);
+            dialog.set_default_response (Response.OPEN);
+        } else {
+            dialog.set_default_response (Response.CANCEL);
+        }
+
+        dialog.show_all ();
+    }
+
+    public Response run () {
+        var res = dialog.run ();
+        dialog.destroy ();
+        return (Response)res;
+    }
+}

=== modified file 'src/View/AbstractDirectoryView.vala'
--- src/View/AbstractDirectoryView.vala	2017-02-15 18:54:50 +0000
+++ src/View/AbstractDirectoryView.vala	2017-03-11 18:39:16 +0000
@@ -795,7 +795,15 @@
                     if (file.is_root_network_folder ()) {
                         load_location (location);
                     } else if (file.is_executable ()) {
-                        file.execute (screen, null, null);
+                        var dialog = new PF.ConfirmExecuteDialog (file, default_app, window);
+                        switch (dialog.run ()) { /* dialog destroys itself */
+                            case PF.ConfirmExecuteDialog.Response.EXECUTE:
+                                file.execute (screen, null, null);
+                                break;
+                            case PF.ConfirmExecuteDialog.Response.OPEN:
+                                open_file (file, screen, default_app);
+                                break;
+                        }
                     } else {
                         open_file (file, screen, default_app);
                     }