Extend File.compare_by_size to use folder item count where appropriate #1978
Conversation
|
Blocking PR was closed without merging. |
# Conflicts fixed in: # libcore/File.vala
|
I would consider this complex enough to require at least some testing especially as it was written a while ago and has had master merged several times. I'll try it out again with some tests from the wiki. |
|
One bug I notice in this is that the item count does not update when files are added (or removed) externally until the view is refreshed. I guess I should fix that. |
|
I don't think there is a simple way of getting the number of items to update automatically - to do that we would have to put a monitor on every folder item. |
|
I have confirmed Nautilus behaves the same. |
|
Another slight quirk is that if a folder contains e.g. just one hidden file then the item count is "1" regardless of the "Show hidden" setting. So opening such a folder can result in the message "Empty" even though the item count is >0. On further investigation this behaviour only occurs if there is just one hidden file in the folder. If there are two files then the item count updates when "Show hidden is toggled". We should probably do the same. |
|
Now only visible items are counted matching Nautilus's behaviour (without the bug). |
|
@zeebok I have made some changes to this PR. I hope you do not mind having another look 😄 |
Co-authored-by: Danielle Foré <danielle@elementary.io>
libcore/File.vala
Outdated
| ensure_item_count (true); // Re-count file items | ||
|
|
||
| if (count < 0) { | ||
| return (_("—")); |
There was a problem hiding this comment.
Does this string really need to be translatable?
There was a problem hiding this comment.
I'll take your advice on this. Does that character have the required meaning in all scripts?
|
Also, it feels strange for me that the item count won't be updated on file creation/deletion inside a folder unless we press Ctrl + R to trigger refresh. |
Co-authored-by: Ryo Nakano <ryonakaknock3@gmail.com>
I can probably get the count to update when the file operation is internal but it still will not if the operation is external. |
|
After the last commit, the count updates after an internal change. It will also update after an external change if the changed folder is open in a tab, window or expanded in list view. It will not update if only being shown as an unexpanded item in a view because there is no Directory object to monitor for changes. This matches Nautilus' behaviour Update: There are some operations like "Paste Into" where the item count is still not updated (but it is in Nautilus). A more complicated solution is needed to cover situations where there is no Directory object for the folder where the item count changes. |
|
We should perhaps consider whether we need to throttle the count updates when adding/removing large numbers of files in one operation else it will slow things up. |
|
Converting to draft while investigating performance |
* Ignore duplicate operations (file already added to or removed from hash) * Ignore CHANGES_DONE_HINT event avoids duplicate update * Only update size once per set of changes if actually changed * Assert each change set from same folder * Simplify File.update_size and rename to ensure_size
|
The latest commits improve performance by updating the count only once per batch of file changes. This assumes that each batch refer to the same folder - this needs testing thoroughly. It is certainly true for internally generated changes as the user can only interact with one background folder at a time but may not be true for external changes. Performance is also improved by only signalling the model(s) to add a file if it is not already in the directory file hash. Also, folder item counts now update their count even if there is no Directory object for them. The Directory handling the change now also checks for a Directory object for the grandparent of the file and if it exists notifies that that its item count should update. As before, external changes are only reflected in item counts if there is a Directory object corresponding to the altered folder (which monitors it). |
|
TESTING: Try to generate a set of file changes where the files in the list do not have the same parent folder. The item count of an unexpanded folder item in ListView should always update when its content is changed within the app e.g. in another tab or window howsoever that is done - cut and paste/paste into/drag and drop/undo and redo. External changes should be reflected in the item count of an unexpanded ListView folder item if that folder is also open in another tab or window. External changes should be reflected in the item count of an expanded ListView folder item. Item counts should reflect the visible items - i.e. they should change when "Show hidden" is toggled according to whether the folder contains hidden files. |
Fixes #1971
Fixes #2108