Bug 81127

Created attachment 5909 [details]
Patch to add file/sub-folder count to Konqueror

Adds a file/folder count to Konqueror's File Properties dialog.

The attached patch resolves this wish item. It just needs to be committed to CVS.

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.

Ok. What do you think of it? Is it ready to go in the HEAD CVS yet?

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; }

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.

Created attachment 5911 [details]
Patch to add file/sub-folder count to Konqueror

version 2 - fixes some issues raised by Jan Schaefer.

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().

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 ;-) ).

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.

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

Great thanks - I forgot to remove m_curUrl this morning when I did the patch.

*** Bug 18048 has been marked as a duplicate of this bug. ***