Merge lp:~evfool/pantheon-photos/lp1275402 into lp:~pantheon-photos/pantheon-photos/trunk

[Robert Roth]

Delete the files instead of trashing them, when emptying the Photos trash.
* UI updated to ask confirmation for delete instead of trashing
* updated delete_original_file method to delete the file instead of trashing
* updated Photo class to use the internal delete file method of the base class instead of duplicating the same code to trash/delete the file.

Delete the files instead of trashing them, when emptying the Photos trash.
* UI updated to ask confirmation for delete instead of trashing
* updated delete_original_file method to delete the file instead of trashing
* updated Photo class to use the internal delete file method of the base class instead of duplicating the same code to trash/delete the file.

[Victor Martinez (victored)]

Great work Robert!

There are a couple of cosmetic issues still noticeable in the code base. The code's logic looks good to me.

review: Needs Fixing

[lp:~evfool/pantheon-photos/lp1275402]

2527. By Robert Roth on 2014-06-11

Do not confirm empty trash (lp:1275402)

[Robert Roth (evfool)]

Thanks Victor for the review, I have updated the proposal, removed the confirmations now, and fixed the style issues you have commented.

[Victor Martinez (victored)]

Thanks for the updates Robert, I have tested the branch and it works as expected. The code looks perfect to me.

review: Approve

=== modified file 'src/Dialogs.vala'
--- src/Dialogs.vala	2014-05-19 21:31:34 +0000
+++ src/Dialogs.vala	2014-06-11 05:27:36 +0000
@@ -1297,16 +1297,16 @@
     }
 }
 
-// Returns: Gtk.ResponseType.YES (trash photos), Gtk.ResponseType.NO (only remove photos) and
+// Returns: Gtk.ResponseType.YES (delete photos), Gtk.ResponseType.NO (only remove photos) and
 // Gtk.ResponseType.CANCEL.
 public Gtk.ResponseType remove_from_library_dialog(Gtk.Window owner, string title,
     string user_message, int count) {
-    string trash_action = ngettext("_Trash File", "_Trash Files", count);
+    string delete_action = ngettext("_Delete File", "_Delete Files", count);
     
     Gtk.MessageDialog dialog = new Gtk.MessageDialog(owner, Gtk.DialogFlags.MODAL,
         Gtk.MessageType.WARNING, Gtk.ButtonsType.CANCEL, "%s", user_message);
     dialog.add_button(_("Only _Remove"), Gtk.ResponseType.NO);
-    dialog.add_button(trash_action, Gtk.ResponseType.YES);
+    dialog.add_button(delete_action, Gtk.ResponseType.YES);
 
     // This dialog was previously created outright; we now 'hijack' 
     // dialog's old title and use it as the primary text, along with
@@ -2668,28 +2668,6 @@
     Gee.ArrayList<Video> videos = new Gee.ArrayList<Video>();
     MediaSourceCollection.filter_media(sources, photos, videos);
     
-    string? user_message = null;
-    if ((!photos.is_empty) && (!videos.is_empty)) {
-        user_message = ngettext("This will remove the photo/video from your Shotwell library.  Would you also like to move the file to your desktop trash?\n\nThis action cannot be undone.",
-            "This will remove %d photos/videos from your Shotwell library.  Would you also like to move the files to your desktop trash?\n\nThis action cannot be undone.",
-             sources.size).printf(sources.size);
-    } else if (!videos.is_empty) {
-        user_message = ngettext("This will remove the video from your Shotwell library.  Would you also like to move the file to your desktop trash?\n\nThis action cannot be undone.",
-            "This will remove %d videos from your Shotwell library.  Would you also like to move the files to your desktop trash?\n\nThis action cannot be undone.",
-             sources.size).printf(sources.size);
-    } else {
-        user_message = ngettext("This will remove the photo from your Shotwell library.  Would you also like to move the file to your desktop trash?\n\nThis action cannot be undone.",
-            "This will remove %d photos from your Shotwell library.  Would you also like to move the files to your desktop trash?\n\nThis action cannot be undone.",
-             sources.size).printf(sources.size);
-    }
-    
-    Gtk.ResponseType result = remove_from_library_dialog(AppWindow.get_instance(), dialog_title,
-        user_message, sources.size);
-    if (result != Gtk.ResponseType.YES && result != Gtk.ResponseType.NO)
-        return;
-    
-    bool delete_backing = (result == Gtk.ResponseType.YES);
-    
     AppWindow.get_instance().set_busy_cursor();
     
     ProgressDialog progress = null;
@@ -2699,44 +2677,28 @@
         monitor = progress.monitor;
     }
         
-    Gee.ArrayList<LibraryPhoto> not_removed_photos = new Gee.ArrayList<LibraryPhoto>();
-    Gee.ArrayList<Video> not_removed_videos = new Gee.ArrayList<Video>();
-    
     // Remove and attempt to trash.
-    LibraryPhoto.global.remove_from_app(photos, delete_backing, monitor, not_removed_photos);
-    Video.global.remove_from_app(videos, delete_backing, monitor, not_removed_videos);
-    
-    // Check for files we couldn't trash.
-    int num_not_removed = not_removed_photos.size + not_removed_videos.size;
-    if (delete_backing && num_not_removed > 0) {
-        string not_deleted_message = 
-            ngettext("The photo or video cannot be moved to your desktop trash.  Delete this file?",
-                "%d photos/videos cannot be moved to your desktop trash.  Delete these files?",
-                num_not_removed).printf(num_not_removed);
-        Gtk.ResponseType result_delete = remove_from_filesystem_dialog(AppWindow.get_instance(), 
-            dialog_title, not_deleted_message);
-            
-        if (Gtk.ResponseType.YES == result_delete) {
-            // Attempt to delete the files.
-            Gee.ArrayList<LibraryPhoto> not_deleted_photos = new Gee.ArrayList<LibraryPhoto>();
-            Gee.ArrayList<Video> not_deleted_videos = new Gee.ArrayList<Video>();
-            LibraryPhoto.global.delete_backing_files(not_removed_photos, monitor, not_deleted_photos);
-            Video.global.delete_backing_files(not_removed_videos, monitor, not_deleted_videos);
-            
-            int num_not_deleted = not_deleted_photos.size + not_deleted_videos.size;
-            if (num_not_deleted > 0) {
-                // Alert the user that the files were not removed.
-                string delete_failed_message = 
-                    ngettext("The photo or video cannot be deleted.",
-                        "%d photos/videos cannot be deleted.",
-                        num_not_deleted).printf(num_not_deleted);
-                AppWindow.error_message_with_title(dialog_title, delete_failed_message, AppWindow.get_instance());
-            }
-        }
+    LibraryPhoto.global.remove_from_app(photos, true, monitor, null);
+    Video.global.remove_from_app(videos, true, monitor, null);
+
+    // Attempt to delete the files.
+    Gee.ArrayList<LibraryPhoto> not_deleted_photos = new Gee.ArrayList<LibraryPhoto>();
+    Gee.ArrayList<Video> not_deleted_videos = new Gee.ArrayList<Video>();
+    LibraryPhoto.global.delete_backing_files(photos, monitor, not_deleted_photos);
+    Video.global.delete_backing_files(videos, monitor, not_deleted_videos);
+
+    int num_not_deleted = not_deleted_photos.size + not_deleted_videos.size;
+    if (num_not_deleted > 0) {
+        // Alert the user that the files were not removed.
+        string delete_failed_message = 
+            ngettext("The photo or video cannot be deleted.",
+                "%d photos/videos cannot be deleted.",
+                num_not_deleted).printf(num_not_deleted);
+        AppWindow.error_message_with_title(dialog_title, delete_failed_message, AppWindow.get_instance());
     }
     
     if (progress != null)
         progress.close();
     
     AppWindow.get_instance().set_normal_cursor();
-}
\ No newline at end of file
+}

=== modified file 'src/MediaDataRepresentation.vala'
--- src/MediaDataRepresentation.vala	2013-09-19 19:58:38 +0000
+++ src/MediaDataRepresentation.vala	2014-06-11 05:27:36 +0000
@@ -99,11 +99,11 @@
         File file = get_master_file();
         
         try {
-            ret = file.trash(null);
+            ret = file.delete(null);
         } catch (Error err) {
             // log error but don't abend, as this is not fatal to operation (also, could be
             // the photo is removed because it could not be found during a verify)
-            message("Unable to move original photo %s to trash: %s", file.get_path(), err.message);
+            message("Unable to delete original photo %s: %s", file.get_path(), err.message);
         }
         
         // remove empty directories corresponding to imported path, but only if file is located

=== modified file 'src/Photo.vala'
--- src/Photo.vala	2014-05-20 04:08:28 +0000
+++ src/Photo.vala	2014-06-11 05:27:36 +0000
@@ -499,15 +499,7 @@
             }
         }
         
-        if (file != null) {
-            try {
-                ret = file.trash(null);
-            } catch (Error err) {
-                ret = false;
-                message("Unable to move editable %s for %s to trash: %s", file.get_path(), 
-                    to_string(), err.message);
-            }
-        }
+        delete_original_file();
         
         // Return false if parent method failed.
         return base.internal_delete_backing() && ret;