Review Request: Consistent file name sorting in the file browser

[Todd]

> On 2010-02-18 15:50:35, Peter Penz wrote:
> > I did some profiling [1] and the updated diff decreases the performance by around 25 %. Sorting is no bottleneck in Dolphin, but after doing some further checks I found out, that KStringHandler::naturalCompare() must be fixed, not KDirSortFilterProxyModelPrivate::compare(). This is good, as it adds no overhead to the performance at all.
> > 
> > I just have added + committed a unittest for bug 201101 (see kdelibs/kdecore/tests/kstringhandlertest.cpp), which fails currently. So for natural sorting, text.txt must be smaller than text1 (natural sorting should just take care that 1 is smaller than 10 etc.), but this is not the case at the moment.
> > 
> > Although the fix should be quite trivial, I don't have the time today to fix this myself. It would be great if you could have a look on this (kdelibs/kdecore/text/kstringhandler.cpp), but of course if you have other topics on your list I'll have a look on this during the next days.
> > 
> > [1] You can use "valgrind --tool=callgrind dolphin --nofork" and check the output file with kcachegrind
> 
> Todd wrote:
>     I'm not sure I'm following here.  Are you saying that my diff is slowing things down, but it is not the fault of the diff, or are you saying that my diff is slowing things down, so I should make the changes in another library (KStringHandler) instead and keep KDirSortFilterProxyModelPrivate::compare() as it was before?
>     
>     Either way, for fixing kstringhandler.cpp, I am currently trying to get the tab drop working.  If I finish that before you have a chance to work on it, I will do it.  Otherwise you can do it.  It depends on how long it takes to get the tab drop working.  When I'm done with that I'll post a review request here.  If you get around to it before I do so, please post a comment here and go ahead with the fix.  Otherwise I will do it.
> 
> Peter Penz wrote:
>     I wanted to say that the performance of your diff is no problem (although it slows down the code), but after taking a more closer look I came to the conclusion that the fix itself is done on the wrong part of the code.
>     
>     I'll let you know here, if I would start with the patch...
> 
> Todd wrote:
>     I got tab drop working, so I will start on this now.

I fixed it so it passes the unit test.  However, it does not match this output: http://sourcefrog.net/projects/natsort/example-out.txt

This is from the natural sort order code that this is supposed to be based on.  The primary issue is that in this the KDE version 0n < n, while in the link n < 0n (where n is any non-zero number).  This further leads to issues in the kde version where, for instance, pic02000 < pic 03, while pic3 < pic2000.  This may be an intentional difference, but if not I should probably fix it now while I am at it.


- Todd


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


On 2010-02-18 00:50:19, Todd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2988/
> -----------------------------------------------------------
> 
> (Updated 2010-02-18 00:50:19)
> 
> 
> Review request for Dolphin and kdelibs.
> 
> 
> Summary
> -------
> 
> In the KDE file manager, there is an inconsistency when sorting by file names when the files have extensions and when they don't.  So, for example, when there is no extension test1 < test1a < test2 < test 10 < test10a < test20.  But when there is an extension you get something like test1a.txt < test1.txt < test2.txt < test10a.txt < test10.txt < test20.txt.  According to the guide that KDE is using for sorting, http://sourcefrog.net/projects/natsort/, the case without extensions is the correct one.  So what this patch does is first compares the filenames without the extension.  If those don't match, it uses that.  If they do match, it compare the extension.  If there are multiple extensions, it compares each extension in sequence.  If the number of extensions do not match, it treats the file with the fewer extensions as having enough empty extensions to make the two files equal.  This fixes the problem without needing any change to the underlying sorting algorithm.s
> 
> 
> This addresses bug 201101.
>     https://bugs.kde.org/show_bug.cgi?id=201101
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kfile/kdirsortfilterproxymodel.cpp 1091061 
> 
> Diff: http://reviewboard.kde.org/r/2988/diff
> 
> 
> Testing
> -------
> 
> Tried sorting different combinations of names, extensions, and extension numbers.
> 
> 
> Thanks,
> 
> Todd
> 
>