klistviewsearchline

Richard Smith kde at metafoo.co.uk
Sun Jul 3 16:57:44 BST 2005


On Sunday 03 July 2005 04:20, Daniel Teske wrote:
> > I could optimize it, to make less changes to the visibility.
>
> So here it is, the optimal patch...
[...]
> I think it's correct.

I think so too. Comments on the patch follow.

> Index: klistviewsearchline.cpp
> ===================================================================
> --- klistviewsearchline.cpp     (revision 429160)
> +++ klistviewsearchline.cpp     (working copy)
> @@ -152,7 +152,10 @@
>      }
>
>      if(d->keepParentsVisible)
> +    {
> +        //showAllChildren(d->listView->firstChild());
>          checkItemParentsVisible(d->listView->firstChild());
> +    }
>      else
>          checkItemParentsNotVisible();
>

Don't check this in.

> @@ -353,18 +356,100 @@
>      }
>  }
>
> +void KListViewSearchLine::showAllChildren(QListViewItem *item)
> +{
> +    for(; item; item = item->nextSibling())
> +    {
> +        if(!item->isVisible())
> +            item->setVisible(true);
> +        else
> +            if(item->firstChild())
> +                showAllChildren(item->firstChild());
> +    }
> +}
> +

Or this.


>  bool KListViewSearchLine::checkItemParentsVisible(QListViewItem *item)
>  {
>      bool visible = false;
> +    for(; item; item = item->nextSibling())
> +    {
> +        if(!item->isVisible())
>          {
> +            if(item->firstChild()) // has a children
> +            {
> +                if(itemMatches(item, d->search))
> +                {
> +                    visible = true;
> +                    item->setVisible(true);
> +                    checkItemParentsVisible(item->firstChild());
> +                }
> +                else
> +                {
> +                    QListViewItem * f = findFirstHiddenToVisible(item,
> item->firstChild());
> +                    if(f) 
> +                    {
> +                        visible = true;
> +                        item->setVisible(true);

This line is redundant, since findFirstHiddenToVisible does this anyway. It's 
confusing to have this here, since it looks like it makes all the children of 
item visible (whereas it actually does nothing).

> +                        hideUntil(item->firstChild(), f);
> +                    }
> +                }
> +            }
> +            else //no children
> +            {
> +                if(itemMatches(item, d->search))
> +                {
> +                    visible = true;
> +                    item->setVisible(true);
> +                }
> +            }
>          }
> +        else // is Visible
> +        {
> +            if(item->firstChild() && 
checkItemParentsVisible(item->firstChild())
> +                    || itemMatches(item, d->search))

Parentheses here would be good - some people don't memorize which of && and || 
has higher precedence.

> +                visible = true;
> +            else
> +                item->setVisible(false);
> +        }
>      }
>      return visible;
>  }


> ===================================================================
> --- klistview.cpp       (revision 429160)
> +++ klistview.cpp       (working copy)
> @@ -369,7 +369,8 @@
>          QListViewItem *i=item;
>          col=0;
>          item=0;
> -        hide(); // will call focusOutEvent, that's why we set item=0
> before
> +        p->setFocus(); // will call focusOutEvent, that's why we 
> set item=0 before
> +        hide(); 
>          if (commit)
>              emit done(i,c);
>      }

Is this supposed to be part of the same patch? If so, can you explain it?
-- 
Thanks,
Richard




More information about the kde-core-devel mailing list