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

Proposed by Robert Roth on 2014-06-10
Status: Merged
Approved by: Daniel Fore on 2014-06-12
Approved revision: 2527
Merged at revision: 2526
Proposed branch: lp:~evfool/pantheon-photos/lp1275402
Merge into: lp:~pantheon-photos/pantheon-photos/trunk
Diff against target: 155 lines (+24/-70)
3 files modified
src/Dialogs.vala (+21/-59)
src/MediaDataRepresentation.vala (+2/-2)
src/Photo.vala (+1/-9)
To merge this branch: bzr merge lp:~evfool/pantheon-photos/lp1275402
Reviewer Review Type Date Requested Status
Victor Martinez 2014-06-10 Approve on 2014-06-12
Review via email: mp+222606@code.launchpad.net

Commit message

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.

Description of the change

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.

To post a comment you must log in.
Victor Martinez (victored) wrote :

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 updated on 2014-06-11
2527. By Robert Roth on 2014-06-11

Do not confirm empty trash (lp:1275402)

Robert Roth (evfool) wrote :

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

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Dialogs.vala'
2--- src/Dialogs.vala 2014-05-19 21:31:34 +0000
3+++ src/Dialogs.vala 2014-06-11 05:27:36 +0000
4@@ -1297,16 +1297,16 @@
5 }
6 }
7
8-// Returns: Gtk.ResponseType.YES (trash photos), Gtk.ResponseType.NO (only remove photos) and
9+// Returns: Gtk.ResponseType.YES (delete photos), Gtk.ResponseType.NO (only remove photos) and
10 // Gtk.ResponseType.CANCEL.
11 public Gtk.ResponseType remove_from_library_dialog(Gtk.Window owner, string title,
12 string user_message, int count) {
13- string trash_action = ngettext("_Trash File", "_Trash Files", count);
14+ string delete_action = ngettext("_Delete File", "_Delete Files", count);
15
16 Gtk.MessageDialog dialog = new Gtk.MessageDialog(owner, Gtk.DialogFlags.MODAL,
17 Gtk.MessageType.WARNING, Gtk.ButtonsType.CANCEL, "%s", user_message);
18 dialog.add_button(_("Only _Remove"), Gtk.ResponseType.NO);
19- dialog.add_button(trash_action, Gtk.ResponseType.YES);
20+ dialog.add_button(delete_action, Gtk.ResponseType.YES);
21
22 // This dialog was previously created outright; we now 'hijack'
23 // dialog's old title and use it as the primary text, along with
24@@ -2668,28 +2668,6 @@
25 Gee.ArrayList<Video> videos = new Gee.ArrayList<Video>();
26 MediaSourceCollection.filter_media(sources, photos, videos);
27
28- string? user_message = null;
29- if ((!photos.is_empty) && (!videos.is_empty)) {
30- 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.",
31- "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.",
32- sources.size).printf(sources.size);
33- } else if (!videos.is_empty) {
34- 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.",
35- "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.",
36- sources.size).printf(sources.size);
37- } else {
38- 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.",
39- "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.",
40- sources.size).printf(sources.size);
41- }
42-
43- Gtk.ResponseType result = remove_from_library_dialog(AppWindow.get_instance(), dialog_title,
44- user_message, sources.size);
45- if (result != Gtk.ResponseType.YES && result != Gtk.ResponseType.NO)
46- return;
47-
48- bool delete_backing = (result == Gtk.ResponseType.YES);
49-
50 AppWindow.get_instance().set_busy_cursor();
51
52 ProgressDialog progress = null;
53@@ -2699,44 +2677,28 @@
54 monitor = progress.monitor;
55 }
56
57- Gee.ArrayList<LibraryPhoto> not_removed_photos = new Gee.ArrayList<LibraryPhoto>();
58- Gee.ArrayList<Video> not_removed_videos = new Gee.ArrayList<Video>();
59-
60 // Remove and attempt to trash.
61- LibraryPhoto.global.remove_from_app(photos, delete_backing, monitor, not_removed_photos);
62- Video.global.remove_from_app(videos, delete_backing, monitor, not_removed_videos);
63-
64- // Check for files we couldn't trash.
65- int num_not_removed = not_removed_photos.size + not_removed_videos.size;
66- if (delete_backing && num_not_removed > 0) {
67- string not_deleted_message =
68- ngettext("The photo or video cannot be moved to your desktop trash. Delete this file?",
69- "%d photos/videos cannot be moved to your desktop trash. Delete these files?",
70- num_not_removed).printf(num_not_removed);
71- Gtk.ResponseType result_delete = remove_from_filesystem_dialog(AppWindow.get_instance(),
72- dialog_title, not_deleted_message);
73-
74- if (Gtk.ResponseType.YES == result_delete) {
75- // Attempt to delete the files.
76- Gee.ArrayList<LibraryPhoto> not_deleted_photos = new Gee.ArrayList<LibraryPhoto>();
77- Gee.ArrayList<Video> not_deleted_videos = new Gee.ArrayList<Video>();
78- LibraryPhoto.global.delete_backing_files(not_removed_photos, monitor, not_deleted_photos);
79- Video.global.delete_backing_files(not_removed_videos, monitor, not_deleted_videos);
80-
81- int num_not_deleted = not_deleted_photos.size + not_deleted_videos.size;
82- if (num_not_deleted > 0) {
83- // Alert the user that the files were not removed.
84- string delete_failed_message =
85- ngettext("The photo or video cannot be deleted.",
86- "%d photos/videos cannot be deleted.",
87- num_not_deleted).printf(num_not_deleted);
88- AppWindow.error_message_with_title(dialog_title, delete_failed_message, AppWindow.get_instance());
89- }
90- }
91+ LibraryPhoto.global.remove_from_app(photos, true, monitor, null);
92+ Video.global.remove_from_app(videos, true, monitor, null);
93+
94+ // Attempt to delete the files.
95+ Gee.ArrayList<LibraryPhoto> not_deleted_photos = new Gee.ArrayList<LibraryPhoto>();
96+ Gee.ArrayList<Video> not_deleted_videos = new Gee.ArrayList<Video>();
97+ LibraryPhoto.global.delete_backing_files(photos, monitor, not_deleted_photos);
98+ Video.global.delete_backing_files(videos, monitor, not_deleted_videos);
99+
100+ int num_not_deleted = not_deleted_photos.size + not_deleted_videos.size;
101+ if (num_not_deleted > 0) {
102+ // Alert the user that the files were not removed.
103+ string delete_failed_message =
104+ ngettext("The photo or video cannot be deleted.",
105+ "%d photos/videos cannot be deleted.",
106+ num_not_deleted).printf(num_not_deleted);
107+ AppWindow.error_message_with_title(dialog_title, delete_failed_message, AppWindow.get_instance());
108 }
109
110 if (progress != null)
111 progress.close();
112
113 AppWindow.get_instance().set_normal_cursor();
114-}
115\ No newline at end of file
116+}
117
118=== modified file 'src/MediaDataRepresentation.vala'
119--- src/MediaDataRepresentation.vala 2013-09-19 19:58:38 +0000
120+++ src/MediaDataRepresentation.vala 2014-06-11 05:27:36 +0000
121@@ -99,11 +99,11 @@
122 File file = get_master_file();
123
124 try {
125- ret = file.trash(null);
126+ ret = file.delete(null);
127 } catch (Error err) {
128 // log error but don't abend, as this is not fatal to operation (also, could be
129 // the photo is removed because it could not be found during a verify)
130- message("Unable to move original photo %s to trash: %s", file.get_path(), err.message);
131+ message("Unable to delete original photo %s: %s", file.get_path(), err.message);
132 }
133
134 // remove empty directories corresponding to imported path, but only if file is located
135
136=== modified file 'src/Photo.vala'
137--- src/Photo.vala 2014-05-20 04:08:28 +0000
138+++ src/Photo.vala 2014-06-11 05:27:36 +0000
139@@ -499,15 +499,7 @@
140 }
141 }
142
143- if (file != null) {
144- try {
145- ret = file.trash(null);
146- } catch (Error err) {
147- ret = false;
148- message("Unable to move editable %s for %s to trash: %s", file.get_path(),
149- to_string(), err.message);
150- }
151- }
152+ delete_original_file();
153
154 // Return false if parent method failed.
155 return base.internal_delete_backing() && ret;

Subscribers

People subscribed via source and target branches

to all changes: