dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kuriikwsfiltereng.cpp:144
> + int start = 0;
> + QString str;
> + while (match.hasMatch()) {
declare where first used (2 lines down)
> kuriikwsfiltereng.cpp:149
> + userquery.replace(match.capturedStart(0),
> match.capturedLength(0), str);
> + start += match.capturedStart(0) + str.size(); // Move after last
> quote
> + match = qsexpr.match(userquery, start);
Are you sure about the += here?
The old code had '=' and I don't understand the +=, given that capturedStart(0)
is an absolute position in userquery, just like start.
> kuriikwsfiltereng.cpp:150
> + start += match.capturedStart(0) + str.size(); // Move after last
> quote
> + match = qsexpr.match(userquery, start);
> }
this kind of duplicates line 142. Would it be more readable to do like you did
in some other places, like
while ((match = qsexpr.match(userquery, start)).hasMatch())
?
> kuriikwsfiltereng.cpp:247
> + const QRegularExpression
> rangeRe(QStringLiteral("([0-9]*)\\-([0-9]*)"));
> + QRegularExpressionMatch rangeMatch;
> + while ((i < rl.count()) && !found) {
declare where first used
> kuriikwsfiltereng.cpp:322
> + newurl.replace(match.capturedStart(0), match.capturedLength(0),
> v);
> + start += match.capturedStart(0) + v.size();
> + match = reflistRe.match(newurl, start);
same comment as above
> kuriikwsfiltereng.cpp:323
> + start += match.capturedStart(0) + v.size();
> + match = reflistRe.match(newurl, start);
> }
same comment as above
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D26332
To: ahmadsamir, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
