Review Request: Match space-separated search terms individually

Thomas Karpiniec tk at 1.21jiggawatts.net
Tue Nov 2 04:51:30 CET 2010


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

(Updated 2010-11-02 03:51:29.998816)


Review request for Amarok.


Changes
-------

Added const modifier to QString


Summary
-------

With this patch a search string of space-separated words will return rows which match every word in the list. The old behaviour is to match rows containing the entire search string in any one of the fields searched.

This permits combination of terms Firefox-style: things like "beatles abbey" to find all tracks from Abbey Road, or "riv anda" to get "Andalucia" from Riverdance.

My only misgiving is that as implemented it will tokenise the search term on every single attempted row match. As rowMatch() is called both directly in several functions to find matches in SearchProxy.cpp and after a timeout to do the row filtering in SortFilterProxy.cpp, pulling the tokenisation up one level will require either that the tokenisation code is repeated, or that the search-related functions are all modified to use QStringLists instead of a QString. If that is a better way I'll do that, but as it stands rowMatch() is not a particularly small function so the small overhead seems reasonable.


Diffs (updated)
-----

  src/playlist/proxymodels/ProxyBase.cpp b6ff53a 

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


Testing
-------

Tested on a playlist of ~8000 tracks. Performance seems good, matching is accurate and it works just fine with single terms or inputs with extra spaces.


Thanks,

Thomas

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101102/b4ae0d48/attachment.htm 


More information about the Amarok-devel mailing list