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