D29061: [KCharSelect] Minor code optimisation
This revision was automatically updated to reflect the committed changes. ahmadsamir marked an inline comment as done. Closed by commit R236:6a9e93c698b8: [KCharSelect] Minor code optimisation (authored by ahmadsamir). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29061?vs=80886=80917 REVISION DETAIL https://phabricator.kde.org/D29061 AFFECTED FILES CMakeLists.txt src/kcharselectdata.cpp To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29061: [KCharSelect] Minor code optimisation
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH l-general-code-opti (branched from master) REVISION DETAIL https://phabricator.kde.org/D29061 To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29061: [KCharSelect] Minor code optimisation
ahmadsamir marked an inline comment as done. ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kcharselectdata.cpp:870 > Shouldn't this be moved to inside the `if (match.hasMatch())` check? Indeed. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D29061 To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29061: [KCharSelect] Minor code optimisation
ahmadsamir updated this revision to Diff 80886. ahmadsamir added a comment. Only use captured() if there's a match REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29061?vs=80781=80886 BRANCH l-general-code-opti (branched from master) REVISION DETAIL https://phabricator.kde.org/D29061 AFFECTED FILES CMakeLists.txt src/kcharselectdata.cpp To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29061: [KCharSelect] Minor code optimisation
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kcharselectdata.cpp:870
> +const QRegularExpressionMatch match = hexExp.match(s);
> +const QString cap = match.captured(1);
> if (match.hasMatch()) {
Shouldn't this be moved to inside the `if (match.hasMatch())` check?
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D29061
To: ahmadsamir, #frameworks, cfeck, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29061: [KCharSelect] Minor code optimisation
ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, cfeck, dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY - Replace one last foreach with range-for, and set -DQT_NO_FOREACH - More const and static where appropriate - If a capture group is not used with QRegularExpression, better make it a clustering/non-capturing group, less book keeping on PCRE side TEST PLAN make && ctest and a quick search in kcharselecttest still works. REPOSITORY R236 KWidgetsAddons BRANCH l-general-code-opti (branched from master) REVISION DETAIL https://phabricator.kde.org/D29061 AFFECTED FILES CMakeLists.txt src/kcharselectdata.cpp To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
