GNOME Bugzilla – Bug 769383
Progress bar incorrect when some file during copy operation are skipped
Last modified: 2016-08-22 18:04:13 UTC
Hi There seems to be a problem when copying file from one folder to another folder and some of the files are already present in the destination directory and should be skipped. Then the progress bar shows the progress as the total number of files that should be copied, but stops when all files that are actually copied (except the skipped ones) are copied. I assume that the correct behaviour would be that either only the number of files shown in the bar are only the files that are actually copied (excluding the skipped ones), or also advance in the progress bar when a file is skipped. I'm running nautilus 3.20.1-3 on Debian unstable. Should you be unable to reproduce this bug, I can provide screenshots. Erik
*** Bug 769975 has been marked as a duplicate of this bug. ***
Created attachment 333435 [details] [review] file-operations: decrease file count when skipping The status string for copy and move operations currently always displays the total count of source files, which is incorrect in cases where some files are skipped. This commit fixes that by decreasing the file count for every skipped file.
Review of attachment 333435 [details] [review]: LGTM thanks a lot!!
Created attachment 333629 [details] [review] file-operations: update progress on skip Currently, the transfer info of an operation is only modified if it is successful, resulting in a confusing reflection in the UI. This commit fixes that by updating the transfer info when files are skipped.
Razvan and I decided to go the opposite route - advancing the progress.
(In reply to Ernestas Kulik from comment #5) > Razvan and I decided to go the opposite route - advancing the progress. ok, can you explain why this approach vs the other in the commit message?
Created attachment 333683 [details] [review] file-operations: update progress on skip Currently, the transfer info of an operation is only modified if it is successful, resulting in a confusing reflection in the UI. Treating skipped operations as completed and displaying them as such feels more natural, as they are technically completed (i.e. nothing has been done). This commit changes the behavior as such.
Review of attachment 333683 [details] [review]: Code looks good except for the name chosen for the function. ::: src/nautilus-file-operations.c @@ +2083,2 @@ static void +transfer_count_file (GFile *file, transfer_count_add_file? the name is strange, I'm not sure about a better proposal though. We want to express we are going to "complete" it or "add it to the count" but avoiding to sounds like we are adding another file to the operation.
(In reply to Carlos Soriano from comment #8) > Code looks good except for the name chosen for the function. I chose it because of this: https://git.gnome.org/browse/nautilus/tree/src/nautilus-file-operations.c#n2675 But sure, I have no objections to changing it.
Created attachment 333774 [details] [review] file-operations: update progress on skip Currently, the transfer info of an operation is only modified if it is successful, resulting in a confusing reflection in the UI. Treating skipped operations as completed and displaying them as such feels more natural, as they are technically completed (i.e. nothing has been done). This commit changes the behavior as such.
Review of attachment 333774 [details] [review]: Good point in your previous comment, didn't realize that. Yeah let's change it and be better, although consistency is probably the most important, since we are planning on a refactor of this whole file, it's going to be good to have already good proposal for names here. ::: src/nautilus-file-operations.c @@ +2014,2 @@ static void +transfer_file_add_to_count (GFile *file, oh good name, except that what we add is the file to the transfer count info. so I guess transfer_add_file_to_count?
Created attachment 333822 [details] [review] file-operations: update progress on skip Currently, the transfer info of an operation is only modified if it is successful, resulting in a confusing reflection in the UI. Treating skipped operations as completed and displaying them as such feels more natural, as they are technically completed (i.e. nothing has been done). This commit changes the behavior as such.
Review of attachment 333822 [details] [review]: perfect, thanks!
Attachment 333822 [details] pushed as 3f97d75 - file-operations: update progress on skip
Created attachment 333869 [details] [review] file-operations: rename calls to transfer_file_add_to_count The function was renamed without renaming the calls to it.
Review of attachment 333869 [details] [review]: ok, thanks for fixing imediatily!
Comment on attachment 333869 [details] [review] file-operations: rename calls to transfer_file_add_to_count Attachment 333869 [details] pushed as d033e12 - file-operations: rename calls to transfer_file_add_to_count
Review of attachment 333822 [details] [review]: ::: src/nautilus-file-operations.c @@ +2017,3 @@ + TransferInfo *transfer_info) +{ + g_autoptr (GFileInfo) file_info; You need to initialize this to NULL!
(In reply to Razvan Chitu from comment #24) > Review of attachment 333822 [details] [review] [review]: > > ::: src/nautilus-file-operations.c > @@ +2017,3 @@ > + TransferInfo *transfer_info) > +{ > + g_autoptr (GFileInfo) file_info; > > You need to initialize this to NULL! No, I do not. I assign to it from g_file_query_info().
Created attachment 333928 [details] [review] file-operations: zero-initialize pointer The file_info autoptr in transfer_add_file_to_count() is not zero-initialized and the function conditionally returns early, which could result in a crash. This commit fixes that.
(In reply to Ernestas Kulik from comment #25) > No, I do not. I assign to it from g_file_query_info(). Yes, I do.
Review of attachment 333928 [details] [review]: I will try to be more careful with this, it's something that keeps avoiding my eyes. Thanks for fixing!
Attachment 333928 [details] pushed as 7f91af5 - file-operations: zero-initialize pointer