Merge lp:~jeremywootten/pantheon-files/fix-1456202-multiple-operations-one-view into lp:~elementary-apps/pantheon-files/trunk

Proposed by Jeremy Wootten on 2017-03-08
Status: Merged
Approved by: Adam Bieńkowski on 2017-06-04
Approved revision: 2520
Merged at revision: 2573
Proposed branch: lp:~jeremywootten/pantheon-files/fix-1456202-multiple-operations-one-view
Merge into: lp:~elementary-apps/pantheon-files/trunk
Diff against target: 78 lines (+3/-19)
1 file modified
src/View/AbstractDirectoryView.vala (+3/-19)
To merge this branch: bzr merge lp:~jeremywootten/pantheon-files/fix-1456202-multiple-operations-one-view
Reviewer Review Type Date Requested Status
Adam Bieńkowski code 2017-03-08 Approve on 2017-06-03
Dieter Debast (community) Approve on 2017-06-03
Review via email: mp+319354@code.launchpad.net

Commit message

remove the blocks on carrying out more than one copy/cut/paste operation or trash/delete operation on the same view at the same time

Description of the change

This branch removes the blocks on carrying out more than one copy/cut/paste operation or trash/delete operation on the same view at the same time.

While these blocks were presumably originally put in place in the interests of stability, removing has not immediately revealed any regressions.

TO TEST (E.G.):

1: Carry out a slow file operation e.g. copying a fairly large folder in place on a remote filesystem or USB stick.

2: Perform another similar copy paste operation on another (or the same) fairly large folder in the same view before the first operation completes. The destination must be the same open view.

In trunk this is not possible. In this branch, both operations run and complete successfully.

Note: With these blocks removed it is possible to do things like deleting the folder which is being copied while the copy is in progress. However, this does not cause a crash - it just causes the copy operation to fail.

This branch needs careful testing with a range of simultaneous operations on the same view to ensure stability is maintained.

To post a comment you must log in.
Dieter Debast (ddieter) wrote :

I can't find any problems on this branch, unless I forgot to test something (but hopefully not). I tried a variety of copying, cutting, deleting and renaming and it doesn't crash or behave in a strange way.

review: Approve
Adam Bieńkowski (donadigo) wrote :

The code looks good to me.

review: Approve (code)
Adam Bieńkowski (donadigo) wrote :

I can confirm that this is working properly, both operations if long enough are showing in the copy dialog and they finish. I'm merging this.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/View/AbstractDirectoryView.vala'
2--- src/View/AbstractDirectoryView.vala 2017-02-15 18:54:50 +0000
3+++ src/View/AbstractDirectoryView.vala 2017-03-08 17:38:37 +0000
4@@ -225,12 +225,8 @@
5 private GLib.AppInfo default_app;
6 private Gtk.TreePath? hover_path = null;
7
8- private bool can_trash_or_delete = true;
9-
10 /* Rapid keyboard paste support */
11- protected bool pasting_files = false;
12 protected bool select_added_files = false;
13- private HashTable? pasted_files = null;
14
15 public bool renaming {get; protected set; default = false;}
16
17@@ -972,7 +968,6 @@
18 return;
19 }
20
21- view.can_trash_or_delete = true;
22 view.unblock_directory_monitor ();
23 }
24
25@@ -990,12 +985,8 @@
26 * when using keybindings. So we remember if the current selection
27 * was already removed (but the view doesn't know about it yet).
28 */
29- if (!can_trash_or_delete)
30- return;
31-
32 unowned GLib.List<GOF.File> selection = get_selected_files_for_transfer ();
33 if (selection != null) {
34- can_trash_or_delete = false;
35 trash_or_delete_files (selection, true, delete_immediately);
36 }
37 }
38@@ -1222,16 +1213,14 @@
39 return;
40 }
41
42- view.pasting_files = false;
43- if (uris == null || uris.size () == 0)
44+ if (uris == null || uris.size () == 0) {
45 return;
46-
47- view.pasted_files = uris;
48+ }
49
50 Idle.add (() => {
51 /* Select the most recently pasted files */
52 GLib.List<GLib.File> pasted_files_list = null;
53- view.pasted_files.foreach ((k, v) => {
54+ uris.foreach ((k, v) => {
55 if (k is GLib.File)
56 pasted_files_list.prepend (k as File);
57 });
58@@ -1242,9 +1231,6 @@
59 }
60
61 private void on_common_action_paste_into (GLib.SimpleAction action, GLib.Variant? param) {
62- if (pasting_files)
63- return;
64-
65 var file = get_files_for_action ().nth_data (0);
66
67 if (file != null && clipboard.can_paste) {
68@@ -1258,10 +1244,8 @@
69
70 if (target.has_uri_scheme ("trash")) {
71 /* Pasting files into trash is equivalent to trash or delete action */
72- pasting_files = false;
73 call_back = (GLib.Callback)after_trash_or_delete;
74 } else {
75- pasting_files = true;
76 /* callback takes care of selecting pasted files */
77 call_back = (GLib.Callback)after_pasting_files;
78 }

Subscribers

People subscribed via source and target branches

to all changes: