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

[Elvis Stansvik]

2009/10/6 Elvis Stansvik <elvstone at gmail.com>:
> 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!

Alright. I attached an updated patch to the bug report:

https://bugs.kde.org/show_bug.cgi?id=204787

Elvis

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