GNOME Bugzilla – Bug 764842
Dropping file in parent folder on pathbar/sidebar
Last modified: 2021-06-18 15:52:43 UTC
When dropping a file on its parent folder in the pathbar/sidebar a conflict dialog is opened asking whether the file/folder should be replaced. The right action for this should be doing nothing.
Created attachment 325651 [details] [review] file-operations: No conflict for dropping file on pathbar/sidebar When dropping a file on the pathbar/sidebar a conflict dialog is opened even though the file is dropped on it's parent folder. In order to solve this, it was checked if the destination directory is the parent of the dropped file/folder.
Review of attachment 325651 [details] [review]: Hey! Good to have a patch! It would be good if you take a look on how the code is organized in the file you are editing before, so we can avoid to do simple errors like mixing declarations and code. Also, comments inline. I'm not sure doing nothing would be a good approach. ::: libnautilus-private/nautilus-file-operations.c @@ +5276,3 @@ dest = get_target_file (src, dest_dir, *dest_fs_type, same_fs); + GFile *src_parent = g_file_get_parent (src); Don't mix declarations and code. Declarations go on the top part of the block. @@ +5279,3 @@ + char *src_parent_path = g_file_get_path (src_parent); + char *dest_dir_path = g_file_get_path (dest_dir); + int have_conflict = strcmp (src_parent_path, dest_dir_path); This is not how file uris work. We use g_file_equal. @@ +5369,3 @@ } + /* Dropping file on parent folder on pathbar/sidebar */ This code is not only for path bar or sidebar. This code is reached in all kinds of situations. So the comment would be invalid in this way. Here you have to take into account all kind of situations that can lead that the source and destination are the same. For example this happens with any folder movement, if a user does a cut and a paste in the same directory, the same code path is reached. Maybe we could show a really simple dialog with "The source forlder and the destination are the same". In any case, let's ask designers. @@ +5372,3 @@ + else if (have_conflict == 0 && + IS_IO_ERROR (error, EXISTS)) { + g_error_free (error); identation is wrong
Created attachment 325775 [details] [review] file-operations: No conflict for cut & paste in parent directory When a file is dropped on the pathbar/sidebar or for a cut & paste in the file's parent directory a conflict dialog is shown asking whether the file should be replaced or not, although it's asking to overwrite the same file. In order to solve this, before launching the conflict dialog it is checked that the action is a "move" and that the destination directory is the same one with the file's parent folder. If this case occurs, a dialog is shown informing the user that the source folder and the destination are the same.
Review of attachment 325775 [details] [review]: ::: libnautilus-private/nautilus-file-operations.c @@ +5277,3 @@ dest = get_target_file (src, dest_dir, *dest_fs_type, same_fs); + src_parent = g_file_get_parent (src); This is leaked, isn't it?
(In reply to Carlos Soriano from comment #4) > Review of attachment 325775 [details] [review] [review]: > > ::: libnautilus-private/nautilus-file-operations.c > @@ +5277,3 @@ > dest = get_target_file (src, dest_dir, *dest_fs_type, same_fs); > > + src_parent = g_file_get_parent (src); > > This is leaked, isn't it? You're right, I'll fix it.
Created attachment 325777 [details] [review] file-operations: No conflict for cut & paste in parent directory When a file is dropped on the pathbar/sidebar or for a cut & paste in the file's parent directory a conflict dialog is shown asking whether the file should be replaced or not, although it's asking to overwrite the same file. In order to solve this, before launching the conflict dialog it is checked that the action is a "move" and that the destination directory is the same one with the file's parent folder. If this case occurs, a dialog is shown informing the user that the source folder and the destination are the same.
Review of attachment 325777 [details] [review]: Looks almost good now. I didn't catch this one earlier: ::: libnautilus-private/nautilus-file-operations.c @@ +190,3 @@ #define MERGE_ALL _("Merge _All") #define COPY_FORCE _("Copy _Anyway") +#define OK _("Ok") Missing the mnemonic
Created attachment 325782 [details] [review] file-operations: No conflict for cut & paste in parent directory When a file is dropped on the pathbar/sidebar or for a cut & paste in the file's parent directory a conflict dialog is shown asking whether the file should be replaced or not, although it's asking to overwrite the same file. In order to solve this, before launching the conflict dialog it is checked that the action is a "move" and that the destination directory is the same one with the file's parent folder. If this case occurs, a dialog is shown informing the user that the source folder and the destination are the same.
Review of attachment 325782 [details] [review]: Looks fine now, thanks!
Created attachment 325815 [details] screenshot Alexandru kindly provided a screenshot of the dialog, which I'm attaching here. There are a few issues with it: * It's missing a title that says exactly what has happened * There's no full stop after the sentence * "Ok" should be spelt "OK". Close is probably a better button label, though. So you might want to go with something like: "Unable to Move File" "The source and destination locations are the same." The title would obviously have to reflect the action and whether it is a single or multiple files. An even better approach would be to use an in-app notification rather than a message dialog: these are less disruptive and automatically time out. The message in that case would be roughly the same.
Review of attachment 325782 [details] [review]: Based on Allan feedback, this needs work. Thanks Allan for taking a look.
Created attachment 334671 [details] [review] No conflict for cut & paste in parent directory When a file is dropped on the pathbar/sidebar or for a cut & paste in the file's parent directory a conflict dialog is shown asking whether the file should be replaced or not, although it's asking to overwrite the same file. In order to solve this, before launching the conflict dialog it is checked that the action is a "move" and that the destination directory is the same one with the file's parent folder. If this case occurs, an in app notification is shown informing the user that the source folder and the destination are the same.
Review of attachment 334671 [details] [review]: Hey! This is a high level review reading in diagonal, in case you can come with a different implementation. The two biggest problems of this: 1- The operation is done in a different thread, you cannot make gtk calls from it. 2- We should not do any external call from the file operations code if possible. In this case it's directly calling public API of the current window, which is not really good as a code design. Look how the run_simple_dialog is done, is quite independent. I believe we will need something like this, or a external file for in-app notifications that is used both by the window and by the file-operations. In any case, Allan, I think we said to discuss the whole dialogs vs in-apps notification for 3.24. I think we agreed I will come with all the warning dialogs, errors dialogs and in-app notification we currently use, and decide which ones should stay as dialogs or which ones should convert to in-app notification and viceversa. So Alex, let's wait until that is sorted out to decide for this patch too. ::: src/nautilus-file-operations.c @@ +6051,3 @@ + g_error_free (error); + + window = gtk_application_get_active_window (GTK_APPLICATION (g_application_get_default ())); this wont work well in a different thread.
*** Bug 626742 has been marked as a duplicate of this bug. ***
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of Files (nautilus), then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/nautilus/-/issues/ Thank you for your understanding and your help.