Bug 81127 - new feature - Folder properties dialog now displays count of files and folders
Summary: new feature - Folder properties dialog now displays count of files and folders
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: kfile (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
: 18048 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-05-08 07:47 UTC by Tim Edwards
Modified: 2004-05-10 21:07 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to add file/sub-folder count to Konqueror (5.30 KB, patch)
2004-05-08 07:48 UTC, Tim Edwards
Details
Patch to add file/sub-folder count to Konqueror (5.07 KB, patch)
2004-05-08 12:01 UTC, Tim Edwards
Details
Patch to add file/sub-folder count to Konqueror (5.00 KB, patch)
2004-05-09 04:59 UTC, Tim Edwards
Details
Patch to add file/sub-folder count to Konqueror (4.91 KB, patch)
2004-05-10 00:57 UTC, Tim Edwards
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Edwards 2004-05-08 07:47:08 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources
OS:          Linux

The Windows folder properties dialog (and the Nautilus one too if I'm
not mistaken) has a convenient count of the number of files and
sub-folders in the directory. 
I think that this feature is sorely missed in KDE and it's long past
time it was added - after all you can hardly expect the average
"word/email/web browser" user to know open a terminal and issue 'find
path -type f | wc -l' or something like that.

I've included a screenshot of the results as well as the patch.
Comment 1 Tim Edwards 2004-05-08 07:48:30 UTC
Created attachment 5909 [details]
Patch to add file/sub-folder count to Konqueror

Adds a file/folder count to Konqueror's File Properties dialog.
Comment 2 Tim Edwards 2004-05-08 08:02:55 UTC
The attached patch resolves this wish item. It just needs to be committed to CVS.
Comment 3 Jan Schaefer 2004-05-08 09:40:16 UTC
I think you have misunderstood the resolved status.
The reporter in general does not resolve his own bug, and if you say it works for you, the bug was no real bug, but works without any patch.
So I reopen this bug, so we can discuss about it.
Comment 4 Tim Edwards 2004-05-08 09:50:02 UTC
Ok. What do you think of it? Is it ready to go in the HEAD CVS yet?
Comment 5 Jan Schaefer 2004-05-08 10:08:49 UTC
I have some comments to your patch:

1) I do not know if it is a good idea to extend the KDirSize class with methods that do not belong there. Perhaps it would be better to create a complete new class, say KDirInfo or something like that. However, this should be decided by the kdelibs developers.

2) The strings you shown on the dialog should be enclosed with 18n(). 

3) It should be sub-folder(s) instead of sub-directory(ies) to be consistent with the rest of KDE.

4) What do you mean with 'items'? Only the number of files or files+directories? Perhaps it should be 'files' instead.

5) Why did you the following?:
-    for (; it != end; ++it) {
+    for (; it != end; it++) {
and later:
-        for( ; it2 != (*it).end(); it2++ ) {
+        for( ; it2 != (*it).end(); ++it2 ) {

6) That comment is obviously wrong:
+  /**
+   * @return the size we found
+   */
+  KIO::filesize_t totalSubdirs() const { return m_totalSubdirs; }
Comment 6 Tim Edwards 2004-05-08 11:59:31 UTC
2) to 6) are fixed (5 was just from when I was fiddling around trying to get it to work).

As for 1) I think I need some of the kdelibs developers to discuss this with me. With the changes in the patch KDirSize could simply be renamed to KFileInfo or whatever - it has pretty much all it needs. Of course you could keep KDirSize in there for compatability with existing code.
Comment 7 Tim Edwards 2004-05-08 12:01:11 UTC
Created attachment 5911 [details]
Patch to add file/sub-folder count to Konqueror

version 2 - fixes some issues raised by Jan Schaefer.
Comment 8 Tim Edwards 2004-05-09 04:59:28 UTC
Created attachment 5917 [details]
Patch to add file/sub-folder count to Konqueror

Fixed internationalisation of plurals by using the 3 parameter form of i18n().
Comment 9 Michael Pyne 2004-05-09 10:07:43 UTC
It's looking much better.  I only have a few quibbles after reading the whole thing this time. :-)

1. The KDirSize constructor that takes a KFileItemList as its parameter doesn't give a default value to m_curUrl, which would probably be a good idea since it isn't immediately initialized like it is in the other constructor.  I can't tell from reading the code whether it's really a problem, but it's better to be safe than sorry.

2. You edit KDirSize::slotEntries to determine if a file is a directory by using a struct stat.  Wouldn't it be better conceptually to re-use the for loop above in that same function?  I think the corresponding UDSEntry is KIO::UDS_FILE_TYPE.  You can use S_ISDIR on that atom like so (taken from kdelibs/kio/kio/job.cpp:1710):
isDir = S_ISDIR((*it2).m_long);

Since the file type was determined anyways, that would save some code.

Everything else looks fine as far as I can tell, but I'm not a KDE expert (yet ;-) ).
Comment 10 Tim Edwards 2004-05-10 00:57:52 UTC
Created attachment 5927 [details]
Patch to add file/sub-folder count to Konqueror

Removes unnecessary explicit use of stat. Now uses the UDS_FILE_TYPE entry from
the UDSEntryList.
Comment 11 David Faure 2004-05-10 13:00:13 UTC
CVS commit by faure: 

Show file count and subfolder count in properties dialog.
Patch by Tim Edwards <tkedwards@optusnet.com.au> - many thanks!
(I only removed the now unused m_curUrl and added @since to the docu)
CCMAIL: 81127-done@bugs.kde.org


  M +16 -4     kdirsize.cpp   1.14
  M +19 -2     kdirsize.h   1.9
  M +6 -2      kpropertiesdialog.cpp   1.307



Comment 12 Tim Edwards 2004-05-10 16:12:19 UTC
Great thanks - I forgot to remove m_curUrl this morning when I did the patch.
Comment 13 Maksim Orlovich 2004-05-10 21:07:12 UTC
*** Bug 18048 has been marked as a duplicate of this bug. ***