Review Request 111012: KFileItemModelRolesUpdater polishing, step 4: final cleanups

[Emmanuel Pescosta]

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

Ship it!


I tested all 4 patches, works fine for me too. 
No regressions found so far, just noticed a much smoother file browsing experience - Awesome! :)



- Emmanuel Pescosta


On June 13, 2013, 9:27 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111012/
> -----------------------------------------------------------
> 
> (Updated June 13, 2013, 9:27 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> This depends on https://git.reviewboard.kde.org/r/111008/ and https://git.reviewboard.kde.org/r/111009/ and https://git.reviewboard.kde.org/r/111011/
> 
> To keep the other 3 patches as readable as possible, I've kept these final changes for a separate patch.
> 
> (a) With the recent changes, KFileItemModelRolesUpdater::resolveNextPendingRoles() is never called when a preview job is running -> no checks needed any more in that function.
> 
> (b) KFileItemModelRolesUpdater::applyResolvedRoles(): Always check if an icon is stored in the model already, and remove the "m_state == ResolvingSortRole || m_state == PreviewJobRunning" check. Without this change, it could happen that the sort role "type" has been resolved for the item already, i.e., the mime type and the final icon are known when we arrive here, but there is still no icon stored in the model. Hm, now that I think of it, I should maybe have put that change at the beginning of the patch series because we now have a temporary regression between steps 2 and 4.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp e539b45 
> 
> Diff: http://git.reviewboard.kde.org/r/111012/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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