[PATCH] KListViewSearchLine fix

[Ingo Klöcker]

On Thursday 10 June 2004 17:36, Andras Mantia wrote:
> Hi,
>
>  I discovered that KListViewSearchLine is buggy as it shows items
> that are not matched by the search string. The easiest way to see is
> to fire the shortcut configuration dialog in an application (take
> KWrite as an example) and type something, like ("bl"). This should
> match only one item (Block Selection), but matches the Align as
> well...
>  The bug is visible also in KMail, but less often as you need a
> thread with changing subjects. The "Refactoring KCM techology"
> (started at 2004-03-19) from kde-core-devel is a good testing thread.
> Try to search for "khelpcenter".
>  The problem is caused by QListViewItem::setVisible() which seems to
> set the visible flags for some child items as well. The following
> patch makes the hiding a lot more complicated than before as it goes
> through the list several times, until it doesn't find anything that
> needs to be removed. But it's not recursive. ;-) . For me it works in
> the shortcut dialog, but for some reason it doesn't work in KMail.
> Does KMail have it's own implementation?
>
> Anyway, here is the patch. If you know a better way to do the same,
> just throw away.

Okay, here's my proposal. Since d->keepParentsVisible is invariant while 
checkItem runs it's pointless to write a complicated checkItem function 
which handles both cases. Therefore I've created to different functions  
checkItemParentsVisible and checkItemParentsNotVisible.

checkItemParentsNotVisible:
In this case all listview items have to be checked. This is done by 
simply iterating over all items. This function is obviously not 
recursive.

checkItemParentsVisible:
In this case I still use a recursion (but this could of course be 
flattened). The advantage of this implementation is that it first 
checks the children. If any of the children is visible then the parent 
should also be visible and thus itemMatches doesn't have to be tested 
for the parent. checkItemParentsVisible returns true if any of the 
siblings of the item that's passed to this method are visible.

Apart from being much easier to grok than the original code (and also 
than Andras' code, but his code doesn't use a recursion so a comparison 
isn't fair) it's also much more efficient than the original code. And 
of course, it also fixes the bug Andras noticed.

Some figures:
Tested with my kde-core-devel folder which holds 19597 messages, 
searching for 'khelpcenter' and clearing the search. The numbers are 
the number of calls of the respective methods.

code         old code     old code  my code      my code
search       khelpcenter   -        khelpcenter   -
itemMatches  19597        19597     19581         7934
setVisible   19677        84992     19597        19597

setVisible has to be called for all listview items exactly ones. That's 
what my code does. The old code called it for all parents of all 
visible items which explains the huge number in the empty search case.

I didn't profile Andras' code. If someone wants to check his code here 
are some macros that I've used to count the number of calls:

#define CREATE_COUNTER(x) int x ## _cnt=0
#define RESET_COUNTER(x) x ## _cnt=0
#define GET_COUNTER(x) x ## _cnt
#define INC_COUNTER(x) x ## _cnt++
#define SHOW_COUNTER(x) kdDebug(5300) << #x " == " << x ## _cnt << endl

Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: klistviewsearchline_pimcopy.patch
Type: text/x-diff
Size: 3119 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20040610/336affa6/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20040610/336affa6/attachment.sig>