Review Request 112717: Start adopting QCollator

Aleix Pol Gonzalez aleixpol at kde.org
Fri Sep 13 17:12:47 UTC 2013


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

Review request for KDE Frameworks, Frank Reininghaus and Aurélien Gâteau.


Description
-------

Now that QCollator is public API, I wanted to give it a try, so I decided to port all uses KStringHandler::naturalCompare() to QCollator.

All the porting was quite straightforward I'd say, the only weird part is that I removed some tests of the KStringHandler tests. There are 2 kind of tests disabled:
- The ones that say that "A" == "a" in case of Qt::CaseInsensitive. ICU is deterministic and it will decide itself which one goes in, so the test doesn't make sense anymore.
- There's a mention to the 237788 bug where in some cases our former algorithm wouldn't be deterministic. This doesn't apply anymore, but also now ICU takes care about it now, so there's little point of keeping unit testing it.
I decided to leave the unit test because it might be useful eventually (although note that it was not being compiled so far). In any case we probably want it out.

In any case, the rest seems straightforward enough. I didn't concentrate on performance though, in some cases we'll want to use the QCollatorSortKey.


Diffs
-----

  KDE5PORTING.html 1287de7 
  kfile/kdirsortfilterproxymodel.cpp 7c7aa5f 
  kfile/kurlnavigatorbutton.cpp d2c27fd 
  staging/itemviews/src/CMakeLists.txt 353a413 
  staging/itemviews/src/kcategorizedsortfilterproxymodel.h a21e7ca 
  staging/itemviews/src/kcategorizedsortfilterproxymodel.cpp c8b652d 
  staging/itemviews/src/kcategorizedsortfilterproxymodel_p.h eb1a67b 
  staging/kcompletion/src/kcompletion.cpp 5f60a6c 
  staging/xmlgui/src/kshortcutsdialog_p.h ab102bc 
  staging/xmlgui/src/kshortcutseditoritem.cpp e89a8aa 
  tier1/kcoreaddons/autotests/CMakeLists.txt 19227dc 
  tier1/kcoreaddons/autotests/kstringhandlertest.cpp d12f086 
  tier1/kcoreaddons/src/lib/text/kstringhandler.h 1b08f6f 
  tier1/kcoreaddons/src/lib/text/kstringhandler.cpp 2f192aa 

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


Testing
-------

Builds, affected tests pass.


Thanks,

Aleix Pol Gonzalez

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130913/e8ab2a30/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list