D26123: Port QRegExp to QRegularExpression

David Faure noreply at phabricator.kde.org
Fri Dec 20 23:36:41 GMT 2019


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kemailaddresstest.cpp:345
>      QFETCH(bool, expResult);
> -
>      QCOMPARE(isValidSimpleAddress(input), expResult);

unrelated and the empty line made sense, please revert

> kemailaddress.cpp:631
> +    // to match the whole subject string
> +    QString addrRx(QStringLiteral("\\A"));
>  

Message from Giuseppe D'Angelo  (author of QRegularExpression) : if you can depend from Qt 5.12, there's QRegularExpression::anchoredPattern instead of manual wrapping in \A and \z

We can't require 5.12 yet but this could be a TODO to simplify this code slightly at some point.

> kemailaddress.cpp:639
>      if (domainPart[ 0 ] == QLatin1Char('[') || domainPart[ domainPart.length() - 1 ] == QLatin1Char(']')) {
> -        addrRx += QStringLiteral("\\[[0-9]{,3}(\\.[0-9]{,3}){3}\\]");
> +        addrRx += QStringLiteral("\\[([0-9]{1,3}\\.){1,3}([0-9]{1,3})\\]");
>      } else {

`{,3}` would be `{0,3}`, not `{1,3}`

Would this allow to minimize the amount of changes made here? I'm worried about regressions.

> kemailaddress.cpp:1119
>  
> -    QRegExp needQuotes(QStringLiteral("[^ 0-9A-Za-z\\x0080-\\xFFFF]"));
> +    QRegularExpression needQuotes(QStringLiteral("[^ 0-9A-Za-z\\x0080-\\xFFFF]"));
>      // avoid double quoting

Also from Giuseppe : the `\\x0080-\\xFFFF` requires changes to `\\x{0080}-\\x{FFFF}`

REPOSITORY
  R270 KCodecs

REVISION DETAIL
  https://phabricator.kde.org/D26123

To: ahmadsamir, #frameworks, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191220/d3d7a963/attachment.html>


More information about the Kde-frameworks-devel mailing list