After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769383 - Progress bar incorrect when some file during copy operation are skipped
Progress bar incorrect when some file during copy operation are skipped
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: File and Folder Operations
3.20.x
Other Linux
: Normal minor
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 769975 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-08-01 14:44 UTC by Erik
Modified: 2016-08-22 18:04 UTC
See Also:
GNOME target: ---
GNOME version: 3.19/3.20


Attachments
file-operations: decrease file count when skipping (2.19 KB, patch)
2016-08-16 16:41 UTC, Ernestas Kulik
accepted-commit_now Details | Review
file-operations: update progress on skip (2.91 KB, patch)
2016-08-19 14:29 UTC, Ernestas Kulik
none Details | Review
file-operations: update progress on skip (3.02 KB, patch)
2016-08-19 17:51 UTC, Ernestas Kulik
none Details | Review
file-operations: update progress on skip (3.07 KB, patch)
2016-08-20 22:01 UTC, Ernestas Kulik
none Details | Review
file-operations: update progress on skip (3.07 KB, patch)
2016-08-21 15:01 UTC, Ernestas Kulik
needs-work Details | Review
file-operations: rename calls to transfer_file_add_to_count (1.82 KB, patch)
2016-08-22 08:45 UTC, Ernestas Kulik
committed Details | Review
file-operations: zero-initialize pointer (1.04 KB, patch)
2016-08-22 16:45 UTC, Ernestas Kulik
committed Details | Review

Description Erik 2016-08-01 14:44:41 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
Comment 1 Ernestas Kulik 2016-08-16 14:21:37 UTC
*** Bug 769975 has been marked as a duplicate of this bug. ***
Comment 2 Ernestas Kulik 2016-08-16 16:41:14 UTC
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.
Comment 3 Carlos Soriano 2016-08-19 10:11:21 UTC
Review of attachment 333435 [details] [review]:

LGTM thanks a lot!!
Comment 4 Ernestas Kulik 2016-08-19 14:29:02 UTC
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.
Comment 5 Ernestas Kulik 2016-08-19 14:30:04 UTC
Razvan and I decided to go the opposite route - advancing the progress.
Comment 6 Carlos Soriano 2016-08-19 17:24:10 UTC
(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?
Comment 7 Ernestas Kulik 2016-08-19 17:51:36 UTC
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.
Comment 8 Carlos Soriano 2016-08-20 17:52:46 UTC
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.
Comment 9 Ernestas Kulik 2016-08-20 18:08:11 UTC
(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.
Comment 10 Ernestas Kulik 2016-08-20 22:01:20 UTC
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.
Comment 11 Carlos Soriano 2016-08-21 13:49:46 UTC
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?
Comment 12 Ernestas Kulik 2016-08-21 15:01:49 UTC
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.
Comment 13 Carlos Soriano 2016-08-21 23:12:46 UTC
Review of attachment 333822 [details] [review]:

perfect, thanks!
Comment 14 Ernestas Kulik 2016-08-22 08:19:21 UTC
Attachment 333822 [details] pushed as 3f97d75 - file-operations: update progress on skip
Comment 15 Ernestas Kulik 2016-08-22 08:45:08 UTC
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.
Comment 16 Carlos Soriano 2016-08-22 08:46:27 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 17 Carlos Soriano 2016-08-22 08:48:32 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 18 Carlos Soriano 2016-08-22 08:48:41 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 19 Carlos Soriano 2016-08-22 08:48:41 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 20 Carlos Soriano 2016-08-22 08:48:42 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 21 Carlos Soriano 2016-08-22 08:48:42 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 22 Carlos Soriano 2016-08-22 08:48:42 UTC
Review of attachment 333869 [details] [review]:

ok, thanks for fixing imediatily!
Comment 23 Ernestas Kulik 2016-08-22 08:50:38 UTC
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
Comment 24 Razvan Chitu 2016-08-22 12:40:01 UTC
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!
Comment 25 Ernestas Kulik 2016-08-22 16:34:07 UTC
(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().
Comment 26 Ernestas Kulik 2016-08-22 16:45:16 UTC
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.
Comment 27 Ernestas Kulik 2016-08-22 16:46:55 UTC
(In reply to Ernestas Kulik from comment #25)
> No, I do not. I assign to it from g_file_query_info().

Yes, I do.
Comment 28 Carlos Soriano 2016-08-22 17:59:58 UTC
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!
Comment 29 Ernestas Kulik 2016-08-22 18:04:08 UTC
Attachment 333928 [details] pushed as 7f91af5 - file-operations: zero-initialize pointer