Review Request 109457: Group files by name correctly also for non-ASCII file names

Frank Reininghaus frank78ac at googlemail.com
Tue Mar 12 18:11:41 GMT 2013


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

Review request for Dolphin.


Description
-------

The existing "Group by Name" code has some issues:

1. In some locales, there are non-ASCII letters which are sorted next to a letter in the range 'A'-'Z' (such as umlauts in the German locale). In the test folder shown in the screenshot, this works fine for A and Ä, which are both shown in the group 'A'. However, Ö is shown in a group 'Others' because there was no file whose name starts with an 'O' before that (there is a TODO comment in the existing code about this issue).

2. All other non-ASCII letters are shown in a single group 'Others', see bug 315569.

I'm proposing a patch to fix these issues.

If anyone wonders why I used STL, rather than Qt containers: I've read in several places that the Qt container templates expand to more code in the executable. They are of course still preferable if one can make good use of their implicit sharing/copy-on-write feature, e.g., if the container is used in a foreach (...) loop, which always takes a copy of the container.

I actually checked how much code is generated in this case by copying the code related to the lettersAtoZ container to a single .cpp file, implementing it once with Qt containers and once with STL containers, and found that with -O2, the 'STL' object file is about 25% smaller. Considering that the STL containers are also faster because they save the indirect access through the pointer to the shared data, I think that it makes sense to use STL containers in new code if we are sure that implicit sharing has no benefits there. This is definitely the case if we are dealing with a local variable in a function, and this variable is never copied and not used in a foreach loop.


This addresses bug 315569.
    http://bugs.kde.org/show_bug.cgi?id=315569


Diffs
-----

  dolphin/src/kitemviews/kfileitemmodel.cpp 7927245 

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


Testing
-------

Grouping works better now for non-ASCII file names, see screenshot.

It would definitely be good if this could be tested by people using other locales and file names with language-specific letters to make sure that it does not break anything.


File Attachments
----------------

Before/after screenshots, made with the de_DE locale
  http://git.reviewboard.kde.org/media/uploaded/files/2013/03/12/Grouping.png


Thanks,

Frank Reininghaus

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


More information about the kfm-devel mailing list