D27914: [Kilpper] Port QRegExp to QRegularExpression
This revision was automatically updated to reflect the committed changes. Closed by commit R120:bcfd7f28370d: [Kilpper] Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27914?vs=78634=79040 REVISION DETAIL https://phabricator.kde.org/D27914 AFFECTED FILES klipper/clipcommandprocess.cpp klipper/configdialog.cpp klipper/editactiondialog.cpp klipper/klipperpopup.cpp klipper/popupproxy.cpp klipper/popupproxy.h klipper/urlgrabber.cpp klipper/urlgrabber.h To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir added inline comments.
INLINE COMMENTS
> apol wrote in urlgrabber.h:185
> I think it's a silly way to do the port but okay, probably works and we get
> to move on.
What is silly about it?
The original code in ClipCommandProcess used:
const QStringList matches = action.regExpMatches();
which was:
QStringList regExpMatches() const { return m_myRegExp.capturedTexts(); }
REPOSITORY
R120 Plasma Workspace
BRANCH
l-klipper (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D27914
To: ahmadsamir, #plasma, davidedmundson, apol, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
apol accepted this revision. apol added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > ahmadsamir wrote in urlgrabber.h:185 > The code stores the QRegularExpression::capturedTexts(), > m_regexCapturedTexts, which is eventually what ClipCommandProcess uses. I think it's a silly way to do the port but okay, probably works and we get to move on. REPOSITORY R120 Plasma Workspace BRANCH l-klipper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Then you want to store a match? The code stores the QRegularExpression::capturedTexts(), m_regexCapturedTexts, which is eventually what ClipCommandProcess uses. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
apol added inline comments. INLINE COMMENTS > ahmadsamir wrote in urlgrabber.h:185 > Another point (which I forgot since it has been some time since I last looked > at this patch), QRegExp stores the match info in itself, whereas > QRegularExpression stores the match info in a QRegularExpressionMatch object > (returned from calling QRegularExpression::match()), so if I use a > m_regularExpression I'll have to add a getter for it, so that it can be > called from URLGrabber::matchingActions(). Then you want to store a match? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Then call it m_regularExpression? Another point (which I forgot since it has been some time since I last looked at this patch), QRegExp stores the match info in itself, whereas QRegularExpression stores the match info in a QRegularExpressionMatch object (returned from calling QRegularExpression::match()), so if I use a m_regularExpression I'll have to add a getter for it, so that it can be called from URLGrabber::matchingActions(). REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
apol added inline comments. INLINE COMMENTS > ahmadsamir wrote in urlgrabber.h:185 > I find it confusing to call the pattern "regExp" and the QRegExp object > "m_myRegExp". Then call it m_regularExpression? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir added inline comments. INLINE COMMENTS > apol wrote in urlgrabber.h:185 > Why did you change how it works right here? I find it confusing to call the pattern "regExp" and the QRegExp object "m_myRegExp". REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
apol added inline comments. INLINE COMMENTS > urlgrabber.h:185 > private: > - QRegExp m_myRegExp; > + QString m_regexPattern; > + QStringList m_regexCapturedTexts; Why did you change how it works right here? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 78634. ahmadsamir added a comment. Rebase; and friendly ping REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27914?vs=77170=78634 BRANCH l-klipper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27914 AFFECTED FILES klipper/clipcommandprocess.cpp klipper/configdialog.cpp klipper/editactiondialog.cpp klipper/klipperpopup.cpp klipper/popupproxy.cpp klipper/popupproxy.h klipper/urlgrabber.cpp klipper/urlgrabber.h To: ahmadsamir, #plasma, davidedmundson, apol, broulik Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir added a comment. Ping. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D27914 To: ahmadsamir, #plasma Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
D27914: [Kilpper] Port QRegExp to QRegularExpression
ahmadsamir created this revision. ahmadsamir added a reviewer: Plasma. Herald added a project: Plasma. ahmadsamir requested review of this revision. TEST PLAN make && ctest klipper still seems to work REPOSITORY R120 Plasma Workspace BRANCH l-klipper (branched from master) REVISION DETAIL https://phabricator.kde.org/D27914 AFFECTED FILES klipper/clipcommandprocess.cpp klipper/configdialog.cpp klipper/editactiondialog.cpp klipper/klipperpopup.cpp klipper/popupproxy.cpp klipper/popupproxy.h klipper/urlgrabber.cpp klipper/urlgrabber.h To: ahmadsamir, #plasma Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
