Review Request 111721: Ensure that the sorting is correct after renaming if the items are not sorted by name, but the name is used as a fallback

Commit Hook null at kde.org
Wed Aug 14 23:10:35 BST 2013


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

(Updated Aug. 14, 2013, 10:10 p.m.)


Status
------

This change has been marked as submitted.


Review request for Dolphin.


Description
-------

1. mkdir test && cd test && touch a b c && dolphin .
2. Sort by size -> the order of the items is "a b c" because the name is used as a fallback (all files have zero size).
3. Rename "a" -> "d".
4. Note that the items are not resorted.

The problem is that KFileItemModel::setData() only re-sorts the model if the "sort role" is changed. However, in cases like the one I described above, also the name matters.

This can be fixed by triggering the re-sorting if either the "sort role" or the name changes. This could in principle cause some unnecessary calls of resortAllItems(). Therefore, I've added a quick check which verifies if the file is still sorted correctly with respect to its neighbors. (A nice side-effect is that renaming files in "Sort by name" mode only triggers resortAllItems() if this is really necessary).


Diffs
-----

  dolphin/src/kitemviews/kfileitemmodel.cpp 1b4911d 
  dolphin/src/tests/kfileitemmodeltest.cpp 513ecef 

Diff: http://git.reviewboard.kde.org/r/111721/diff/


Testing
-------

Old and new tests pass, no regressions seen so far. Hm, thinking about it again, there is a similar bug in KFileItemModel::slotRefreshItems() - can be seen quite easily by applying the patch, repeating the steps above and splitting the view after step 2. I'll have a look at that too, but it doesn't hurt to do it in two different commits anyway.


Thanks,

Frank Reininghaus

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


More information about the kfm-devel mailing list