Group files by name correctly also for non-ASCII file names

Review Request #109457 - Created March 12, 2013 and submitted

Information
Frank Reininghaus
kde-baseapps
KDE/4.10
315569
Reviewers
dolphin
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.
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.

Files


Commit Hook
Frank Reininghaus
Review request changed

Status: Closed (submitted)

Loading...