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

[Elvis Stansvik]

2009/10/6 Pino Toscano <pino at kde.org>:
> Hi,
>
> really sorry it took so long, finally we are at it.

No problem ;)

>
>> 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/

I've switched distro to Arch since I made the patch, so I need to fix
up my dev env. But after that I'll fix these issues and post a new
patch. Thanks for the input!

>
> 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 ;) ).

Yea, I thought about that. I'll try to make a patch against kdelibs
and propose it sometime. Kind of busy with school assignments at the
moment ;)

Elvis

>
> Thanks,
> --
> Pino Toscano
>
> _______________________________________________
> Okular-devel mailing list
> Okular-devel at kde.org
> https://mail.kde.org/mailman/listinfo/okular-devel
>
>