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