2009/10/6 Pino Toscano <p...@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@kde.org > https://mail.kde.org/mailman/listinfo/okular-devel > > _______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel