ahmadsamir added a comment.
FTR; in one of my inline comments I said:
> And I found that:
>
> - {,3} in QRegExp is equivalent to {0,3}
> - {,3} in QRegularExpression is equivalent to {1,3}
The second part is actually wrong (as I was told by dfaure who was told by
upstream QReg
This revision was automatically updated to reflect the committed changes.
Closed by commit R270:7310a343212a: Port QRegExp to QRegularExpression
(authored by ahmadsamir).
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=72139&id=72141
REVISION DETAIL
ahmadsamir updated this revision to Diff 72139.
ahmadsamir marked an inline comment as done.
ahmadsamir edited the summary of this revision.
ahmadsamir added a comment.
Fix coding style
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=72045&id=72139
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Thanks for the extensive research!
INLINE COMMENTS
> kemailaddress.cpp:608
>
> bool tooManyAtsFlag = false;
> bool inQuotedString = false;
[pre-existing: this bool is never set
ahmadsamir updated this revision to Diff 72045.
ahmadsamir added a comment.
Use const
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=71999&id=72045
BRANCH
l-qregexp (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D26123
AF
ahmadsamir updated this revision to Diff 71999.
ahmadsamir added a comment.
Verbatim
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=71941&id=71999
BRANCH
l-qregexp (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D26123
AFF
ahmadsamir updated this revision to Diff 71941.
ahmadsamir added a comment.
Frameworks min. required Qt has just been raised to 5.12
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=71934&id=71941
BRANCH
l-qregexp (branched from master)
REVISION
ahmadsamir added a comment.
Sorry, I did reply to your inline comments but didn't "submit" the replies
only "save draft"ed.
REPOSITORY
R270 KCodecs
REVISION DETAIL
https://phabricator.kde.org/D26123
To: ahmadsamir, #frameworks, dfaure, mlaurent, vkrause
Cc: kde-frameworks-devel, LeGast0
ahmadsamir added inline comments.
INLINE COMMENTS
> dfaure wrote in kemailaddress.cpp:631
> 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 requ
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> dfaure wrote in kemailaddress.cpp:639
> `{,3}` would be `{0,3}`, not `{1,3}`
>
> Would this allow to minimize the amount of changes made here? I'm worried
>
ahmadsamir updated this revision to Diff 71934.
ahmadsamir marked 4 inline comments as done.
ahmadsamir edited the summary of this revision.
ahmadsamir added a comment.
Address review comments
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=71907&id
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
ahmadsamir updated this revision to Diff 71907.
ahmadsamir edited the summary of this revision.
ahmadsamir added a comment.
One more QRegExp
REPOSITORY
R270 KCodecs
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26123?vs=71900&id=71907
BRANCH
l-qregexp (branched from master)
R
ahmadsamir planned changes to this revision.
ahmadsamir added a comment.
I missed one QRegExp.
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, b
ahmadsamir created this revision.
ahmadsamir added reviewers: Frameworks, dfaure, mlaurent, vkrause.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.
REVISION SUMMARY
I had to modify the regex pattern slightly to make kemailaddresstest pass
TEST PLAN
make && c
15 matches
Mail list logo
