Review Request 108766: Simplify comparison of children of expanded folders

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108766/#review26790
-----------------------------------------------------------

Ship it!


Tested and works fine. Awesome work! ;)

- Emmanuel Pescosta


On Feb. 3, 2013, 11:10 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108766/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2013, 11:10 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The comparison of items when expanded folders are present, which is done mainly in the function KFileItemModel::expandedParentsCountCompare() is unnecessarily complex. It analyzes the sub-folder structure based on the URLs of both items. This is quite complicated and can cause problems in some situations, see bug 311947.
> 
> I propose to remove most of the current code and just use the 'parent', which is stored in the ItemData for each item, to compare the items. This requires much less code, is faster, and less buggy. 
> 
> This patch consists of three parts:
> 
> 1. The changes in KFileItemModel (including additional checks in KFileItemModel::isConsistent()),
> 2. A new unit test for bug 311947,
> 3. A new benchmark for adding and removing many items in a tree structure.
> 
> Comments welcome!
> 
> 
> This addresses bug 311947.
>     http://bugs.kde.org/show_bug.cgi?id=311947
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.h 903291a 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 1062aa5 
>   dolphin/src/tests/kfileitemmodelbenchmark.cpp 7017863 
>   dolphin/src/tests/kfileitemmodeltest.cpp 2ad1842 
> 
> Diff: http://git.reviewboard.kde.org/r/108766/diff/
> 
> 
> Testing
> -------
> 
> I cannot reproduce the crash any more. Existing tests still work OK. The benchmark for adding and removing many items in a tree structure runs in about 1.7 seconds, rather than 2.9 seconds.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130206/9bfc704b/attachment.htm>