Review Request: Fix for bug 252923 - show number of items in a directory

[Raphael Kubo da Costa]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6167/#review9370
-----------------------------------------------------------


Thanks for the patch. After polishing these rough edges, it can probably go in.

In the original bug report, the reporter mentioned an option to turn this on and off in case it incurred a performance penalty; I've measured it with callgrind while opening and browsing a kdebase tarball and it didn't cause performance problems -- have you performed any kind of test in this regard?


/trunk/KDE/kdeutils/ark/part/archivemodel.h
<http://svn.reviewboard.kde.org/r/6167/#comment10378>

    Ark follows kdelibs' coding style, which in this case means "SomeClass &foo" instead of "SomeClass& foo", so can you please adjust this here and in the other places?



/trunk/KDE/kdeutils/ark/part/archivemodel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10379>

    Minor nitpick. Can you declare a variable per line?



/trunk/KDE/kdeutils/ark/part/archivemodel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10383>

    const int, please.



/trunk/KDE/kdeutils/ark/part/archivemodel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10380>

    Please use false instead of 0 for the last argument.



/trunk/KDE/kdeutils/ark/part/archivemodel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10384>

    You need not use a reference here, as QList is one of Qt's implicitly shared classes. You should const'ify it, though.



/trunk/KDE/kdeutils/ark/part/archivemodel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10385>

    After const'ifying entries, you could use a const ArchiveNode *node here.



/trunk/KDE/kdeutils/ark/part/infopanel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10382>

    const int, please.



/trunk/KDE/kdeutils/ark/part/infopanel.cpp
<http://svn.reviewboard.kde.org/r/6167/#comment10381>

    Ditto about s/0/false.


- Raphael


On 2010-12-19 15:55:27, Ghislain Mary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6167/
> -----------------------------------------------------------
> 
> (Updated 2010-12-19 15:55:27)
> 
> 
> Review request for kdeutils.
> 
> 
> Summary
> -------
> 
> This patch does what the bug report says but it tells how many folders and
> files there are in a folder instead of just the number of items.
> It also changes the info panel to have consistent information with the list
> view.
> 
> 
> This addresses bug 252923.
>     https://bugs.kde.org/show_bug.cgi?id=252923
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeutils/ark/part/archivemodel.h 1207605 
>   /trunk/KDE/kdeutils/ark/part/archivemodel.cpp 1207605 
>   /trunk/KDE/kdeutils/ark/part/infopanel.cpp 1207605 
> 
> Diff: http://svn.reviewboard.kde.org/r/6167/diff
> 
> 
> Testing
> -------
> 
> Tested with some archives contained in the kde source tree.
> 
> 
> Thanks,
> 
> Ghislain
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-utils-devel/attachments/20101222/a134977d/attachment.htm