D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure added a comment. > faster if one msec is even perceivable at all. It's not, but if an application has 200 strings to translate, then there's a 0.2s difference. That starts to be perceivable. REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D26366 To:

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. In D26366#589229 , @dfaure wrote: > So, the old way was 76 times faster than the new regexp :-) > > I'm not surprised, though, it's consistent with my experience with regexps. > > This might be a good reason to

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure added a comment. So, the old way was 76 times faster than the new regexp :-) I'm not surprised, though, it's consistent with my experience with regexps. This might be a good reason to use the manual-search way. Especially now that you tested it for both performance and

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. In D26366#588416 , @dfaure wrote: > Benchmarking is complex ;) > > 1. you need to make sure both Qt and your benchmark are built with optimizations enabled (-O2) > 2. you need to actually use those 3 variables

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. In D26366#588416 , @dfaure wrote: > Benchmarking is complex ;) > > 1. you need to make sure both Qt and your benchmark are built with optimizations enabled (-O2) > 2. you need to actually use those 3 variables

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure added a comment. Benchmarking is complex ;) 1. you need to make sure both Qt and your benchmark are built with optimizations enabled (-O2) 2. you need to actually use those 3 variables otherwise the compiler might optimize out their use. This could be testing their value (NOT

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. I hacked up some (rather crude, to say the least) benchmarking: void KLocalizedStringTest::benchmarkRegexSimple() { QString roleName, cueName, formatName; QString context = QStringLiteral("@info:tooltip/plaintext"); QBENCHMARK

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ilic added inline comments. INLINE COMMENTS > dfaure wrote in kuitmarkup.cpp:90 > Interesting idea, this simplifies the code. > > However the name "wsRx" no longer matches what this regexp does. > > Also, the old trimming is missing here, isn't it? > I guess \s*:?\s* is needed, and \s*/?\s* >

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

This revision was automatically updated to reflect the committed changes. Closed by commit R249:a9f4cbdda790: [Kuit] Port QRegExp to QRegularExpression, third pass (authored by ahmadsamir). REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26366?vs=72748=72763

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R249 KI18n BRANCH l-parseUiM (branched from master) REVISION DETAIL https://phabricator.kde.org/D26366 To: ahmadsamir, #frameworks, ilic, dfaure, mlaurent, aacid Cc: kde-frameworks-devel,

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir updated this revision to Diff 72748. ahmadsamir added a comment. Rebase after the unit tests commits REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26366?vs=72710=72748 BRANCH l-parseUiM (branched from master) REVISION DETAIL

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure added a comment. You just made me realize something. The setter is unused, but the getter is used in all cases, to lookup the builtin formats. So I just extended the unittests in https://commits.kde.org/ki18n/4f9d907fb8e8dba33aa80926d3b36ba12f30da04 These tests do check the

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. In D26366#587313 , @dfaure wrote: > See D26408 for a unittest. But maybe the code can just be killed... My 2p, keep it as it looks like a way to override the default

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure added a comment. See D26408 for a unittest. But maybe the code can just be killed... REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D26366 To: ahmadsamir, #frameworks, ilic, dfaure, mlaurent, aacid Cc: kde-frameworks-devel,

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added a comment. About the unit test, I couldn't think of a way to test it, unless I use a snippet to check the regex (like you did in D26332 ), which doesn't seem to fit in a unit test for KLocalizedString... REPOSITORY R249 KI18n REVISION

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir updated this revision to Diff 72709. ahmadsamir added a comment. Use a meaningful name for the QRegularExpression object Explain the (my) logic behind the changes in parseUiMarker() in the commit message REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir updated this revision to Diff 72710. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Verbatim REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26366?vs=72709=72710 BRANCH l-parseUiM (branched from master) REVISION

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kuitmarkup.cpp:90 > Interesting idea, this simplifies the code. > > However the name "wsRx" no longer matches what this regexp does. > > Also, the old trimming is missing here, isn't it? > I guess \s*:?\s* is needed, and

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kuitmarkup.cpp:90 > > -// Role. > -roleName = context; > +static const QRegularExpression >

D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, ilic, dfaure, mlaurent, aacid. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY Port parseUiMarker() and determineIsStructured(). TEST PLAN make && ctest REPOSITORY