Merge lp:~hyuchia/screenshot-tool/fix-1668074 into lp:~elementary-apps/screenshot-tool/trunk

[Diego Islas Ocampo]

Ensure screenshots are saved to correct directory when launched from Slingshot.

Changed the saving directory to the one available in the settings or the environment's pictures default folder for screenshots taken from the launcher.

[Adam Bieńkowski (donadigo)]

Just left some diff comments.

[lp:~hyuchia/screenshot-tool/fix-1668074]

311. By Diego Islas Ocampo on 2017-03-02

Remove redundant calls

[Diego Islas Ocampo (hyuchia)]

You were right, changing it to owned was not necessary, I've also used a variable instead o calling the function twice. Thanks!

[Jeremy Wootten (jeremywootten)]

I guess it is not strictly needed for this fix, but it would be better to just save the screenshot and deal with any arising GLib.Error (which should be done anyway) - rather than use the rather strange do loop to check for (non)existence.

The branch does work as expected.

[lp:~hyuchia/screenshot-tool/fix-1668074]

312. By Diego Islas Ocampo on 2017-03-21

Show error dialog when save raises an error

313. By Diego Islas Ocampo on 2017-03-21

Add check for save directory existance

[Jeremy Wootten (jeremywootten)]

If save_file gets passed an invalid (non-existent) directory (which could happen when using the Plank context menu method) then the save fails, so it would be best to do an existence check here as well as in the SaveDialog. You can just pass "" as the folder_dir parameter when called with from_command == true, instead of looking up the setting twice.

[lp:~hyuchia/screenshot-tool/fix-1668074]

314. By Diego Islas Ocampo on 2017-03-21

Remove unnecessary directory specificaction

315. By Diego Islas Ocampo on 2017-03-21

Add folder existance check

[Jeremy Wootten (jeremywootten)]

Looks good now :)

review: Approve

=== modified file 'src/ScreenshotWindow.vala'
--- src/ScreenshotWindow.vala	2017-02-18 22:07:14 +0000
+++ src/ScreenshotWindow.vala	2017-03-21 17:14:34 +0000
@@ -348,10 +348,13 @@
 
                     /// TRANSLATORS: %s represents a timestamp here
                     string file_name = _("Screenshot from %s").printf (date_time);
-                    string folder_dir = settings.get_string ("folder-dir");
                     string format = settings.get_string ("format");
-
-                    save_file (file_name, format, folder_dir, screenshot);
+                    try {
+                        save_file (file_name, format, "", screenshot);
+                    } catch (GLib.Error e) {
+                        show_error_dialog ();
+                        debug (e.message);
+                    }
                 }
                 this.destroy ();
             }
@@ -359,21 +362,31 @@
             return false;
         }
 
-        private void save_file (string file_name, string format, string folder_dir, Gdk.Pixbuf screenshot) {
+        private void save_file (string file_name, string format, string folder_dir, Gdk.Pixbuf screenshot) throws GLib.Error {
             string full_file_name = "";
-                int attempt = 0;
-
-                do {
-                    if (attempt == 0) {
-                        full_file_name = Path.build_filename (folder_dir, "%s.%s".printf (file_name, format));
-                    } else {
-                        full_file_name = Path.build_filename (folder_dir, "%s (%d).%s".printf (file_name, attempt, format));
-                    }
-
-                    attempt++;
-                } while (File.new_for_path (full_file_name).query_exists ());
-
-                screenshot.save (full_file_name, format);
+
+            if (folder_dir == "") {
+                string folder_from_settings = settings.get_string ("folder-dir");
+                if (folder_from_settings != "" && File.new_for_path (folder_from_settings).query_exists ()) {
+                    folder_dir = folder_from_settings;
+                } else {
+                    folder_dir = GLib.Environment.get_user_special_dir (GLib.UserDirectory.PICTURES);
+                }
+            }
+
+            int attempt = 0;
+
+            do {
+                if (attempt == 0) {
+                    full_file_name = Path.build_filename (folder_dir, "%s.%s".printf (file_name, format));
+                } else {
+                    full_file_name = Path.build_filename (folder_dir, "%s (%d).%s".printf (file_name, attempt, format));
+                }
+
+                attempt++;
+            } while (File.new_for_path (full_file_name).query_exists ());
+
+            screenshot.save (full_file_name, format);
         }
 
         public void take_clicked () {

=== modified file 'src/Widgets/SaveDialog.vala'
--- src/Widgets/SaveDialog.vala	2017-02-15 21:00:15 +0000
+++ src/Widgets/SaveDialog.vala	2017-03-21 17:14:34 +0000
@@ -45,8 +45,10 @@
 
             var folder_dir = Environment.get_user_special_dir (UserDirectory.PICTURES);
 
-            if (settings.get_string ("folder-dir") != folder_dir && settings.get_string ("folder-dir") != "") {
-                folder_dir = settings.get_string ("folder-dir");
+            var folder_from_settings = settings.get_string ("folder-dir");
+
+            if (folder_from_settings != folder_dir && folder_from_settings != "" && File.new_for_path (folder_from_settings).query_exists ()) {
+                folder_dir = folder_from_settings;
             }
 
             int width = pixbuf.get_width () / 4;