D26435: [Syndication] Port QRegExp to QRegularExpression
David Faure
noreply at phabricator.kde.org
Sun Jan 5 17:49:26 GMT 2020
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
In D26435#587956 <https://phabricator.kde.org/D26435#587956>, @ahmadsamir wrote:
> I look for usage of tagRegExp() with lxr.kde.org and couldn't find any.
A file-static function is not exported outside that cpp file, so that's quite expected.
INLINE COMMENTS
> loaderutil.cpp:69
> QString host = url.host();
> rx.setPattern(QStringLiteral("(?:<A )[^H]*(?)[^=]*=[^A-Z0-9-_~,./]*([^'\">\\s]*)"));
> + QRegularExpressionMatchIterator iter = rx.globalMatch(str);
Indeed that `(?)` looks invalid. kregexpeditor says so, at least.
@mlaurent in commit dda15b0aaddec01 <https://phabricator.kde.org/R96:dda15b0aaddec01e31f862ba486a4e2876ab8eb8> you moved code around, but you also replaced `(?:HREF)` with `(?)`, while the rest of this regexp didn't change. Any reason for that?
I think that either HREF should be added back, or `(?)` should be removed.
> tools.cpp:159
> //TODO: preserve some formatting, such as line breaks
> - str.remove(QRegExp(QStringLiteral("<[^>]*>"))); // remove tags
> + str.remove(QRegularExpression(QStringLiteral("<[^>]*?>"))); // remove tags
> str = resolveEntities(str);
Any reason for the added ? (for non-greedy-match)? I can't think of a case where it would make a difference, here.
REPOSITORY
R96 PIM: Syndication Support
REVISION DETAIL
https://phabricator.kde.org/D26435
To: ahmadsamir, dfaure, mlaurent
Cc: kde-pim, fbampaloukas, dcaliste, dvasin, rodsevich, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200105/a6a942e0/attachment.html>
More information about the kde-pim
mailing list