Review Request 111808: Introduce a new signal for the case that the groups have changed, even though no items have been moved at all

Emmanuel Pescosta emmanuelpescosta099 at gmail.com
Fri Aug 2 15:55:46 BST 2013


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

Ship it!


Crash in KItemListSizeHintResolver::itemsMoved - out of range error! :( 
When I change the sorting type if grouping is enabled.

You can fix it by replacing the line "const int newIndex = movedToIndexes.at(i);" with "const int newIndex = movedToIndexes.at(i - range.index);" in KItemListSizeHintResolver::itemsMoved, because the moveToIndexes list starts from 0 and not from range.index.

But everything else works fine :) Ship it after you have fixed the out of range error!

- Emmanuel Pescosta


On July 30, 2013, 10:08 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111808/
> -----------------------------------------------------------
> 
> (Updated July 30, 2013, 10:08 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> This is based on https://git.reviewboard.kde.org/r/111807/.
> 
> This patch aims to remove the need to emit the "itemsMoved" signal if no items have been moved at all. Up to now, the view still needs this signal to adapt to the (possibly changed) groups. I think it's better to emit a new signal in that case.
> 
> The benefits of this patch include:
> * Can improve the performance: no expensive updates needed if no items have been moved after a resorting.
> * Allows to re-enable some unit tests which have been disabled some time ago.
> * Makes it easier to find a better version of https://git.reviewboard.kde.org/r/111721/
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 1b4911d 
>   dolphin/src/kitemviews/kitemlistview.h 6467b8c 
>   dolphin/src/kitemviews/kitemlistview.cpp 0c3183c 
>   dolphin/src/kitemviews/kitemmodelbase.h 70f6883 
>   dolphin/src/tests/kfileitemmodeltest.cpp 0ad7a37 
> 
> Diff: http://git.reviewboard.kde.org/r/111808/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass. Haven't seen any regressions so far.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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


More information about the kfm-devel mailing list