[Okular-devel] [PATCH] Support regular expressions and case sensitivity in TOC search input

Pino Toscano pino at kde.org
Tue Oct 6 16:52:34 CEST 2009


Hi,

really sorry it took so long, finally we are at it.

> Attached is a patch for review that adds a context submenu to the
> search line called "Search Options" where you can enable regular
> expression support and case sensitivity. Also attaching a screen shot
> to show what it looks like.

Looks nice, just few notes below:

>    if ( !d->searchColumns.isEmpty() ) {
>      QList<int>::ConstIterator it = d->searchColumns.constBegin();
>      for ( ; it != d->searchColumns.constEnd(); ++it ) {
> -      if ( *it < columncount &&
> -           index.child( row, *it ).data( Qt::DisplayRole
>  ).toString().indexOf( pattern, 0, d->caseSensitive ) >= 0 )
>  -        return true;
> +      if ( *it < columncount ) {
> +        if ( d->regularExpression ) {
> +          return (index.child( row, *it ).data( Qt::DisplayRole
>  ).toString().indexOf( +              QRegExp( pattern, d->caseSensitive ))
>  >= 0);
> +        } else {
> +          return (index.child( row, *it ).data( Qt::DisplayRole
>  ).toString().indexOf( +              pattern, 0, d->caseSensitive ) >= 0);
> +        }
> +      }
>      }
>    } else {
>      for ( int i = 0; i < columncount; ++i) {
> -      if ( index.child( row, i ).data( Qt::DisplayRole
>  ).toString().indexOf( pattern, 0, d->caseSensitive ) >= 0 )
>  -        return true;
> +      if ( d->regularExpression ) {
> +        return (index.child( row, i ).data( Qt::DisplayRole
>  ).toString().indexOf( +            QRegExp( pattern, d->caseSensitive ))
>  >= 0);
> +      } else {
> +        return (index.child( row, i ).data( Qt::DisplayRole
>  ).toString().indexOf( +            pattern, 0, d->caseSensitive ) >= 0);
> +      }
>      }
>    }

We could simplify (and speedup a bit) this part: instead of checking if it's a 
regular expression search or not (and construct a new QRegExp object each 
time), a QRegExp object could always be constructed once before the loop, 
setting the correct case sensitivity and syntax (regexp or plain) for it.
Also, instead of using QString::indexOf(QRegExp), please use 
QRegExp::indexIn(); this because the former is highly unsafe, see
  http://labs.trolltech.com/blogs/2008/11/04/910/

Also, as this class is a fork+adaptation from kdelibs KTreeWidgetSearchLine 
(see kdelibs/kdeui/itemviews/ktreewidgetsearchline.cpp), if you can/want feel 
free to also propose the additions for that class as well, so other KDE 
applications using it can benefit from the additions (and simplify future 
merges from it ;) ).

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/okular-devel/attachments/20091006/1fff89aa/attachment.sig 


More information about the Okular-devel mailing list