Review Request: Match space-separated search terms individually

[Leo Franchi]

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

Ship it!


I think he meant const ref, so const QString&. But I'll do it before committing--patch looks good!

- Leo


On 2010-11-02 03:51:29, Thomas Karpiniec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100128/
> -----------------------------------------------------------
> 
> (Updated 2010-11-02 03:51:29)
> 
> 
> Review request for Amarok.
> 
> 
> 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
> -----
> 
>   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/20101106/da9114f2/attachment-0001.htm