D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
dfaure added a comment. Fixed, thanks REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: iasensio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
iasensio added a comment. Hi! This change fails to build for me, and also in CI (https://build.kde.org/view/Failing/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.12/409/console) kcoredirlister_p.h:148:37: error: field 'lstFilters' has incomplete type 'QVector' 09:29:39148 | QVector lstFilters; I just needed to `#include ` to fix it REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: iasensio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73989. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Rebase and tweak the commit message REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73796=73989 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
This revision was automatically updated to reflect the committed changes. Closed by commit R241:df383663d14e: [KCoreDirLister] Port QRegExp to QRegularExpression (authored by ahmadsamir). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73989=73990 REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73796. ahmadsamir added a comment. - Don't use anchoredPattern() with wilcardToRegularExpression() as the latter already returns an anchored pattern - Remove the bit about setNameFilter() from the commit message, wildcardToRegularExpression() should work with file glob patterns REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73676=73796 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kcoredirlister.cpp:2306 > I think you said this would anchor twice, in another review? Needs to be > fixed then. > > (Good for readability!) Right. (I searched through all the porting commits, I missed this one as it hasn't committed yet...). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcoredirlister.cpp:2306 > +d->settings.lstFilters.append(QRegularExpression( > + > QRegularExpression::anchoredPattern(QRegularExpression::wildcardToRegularExpression(filter)), > +QRegularExpression::CaseInsensitiveOption)); I think you said this would anchor twice, in another review? Needs to be fixed then. (Good for readability!) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73676. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Add TODO KF6 notes to remove doNameFilter() and doMimeFilter() REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73605=73676 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ahmadsamir wrote in kcoredirlister.h:595 > Thanks, fixed. Luckily it's a convenience function of sorts and replacing the > one instance it was used in KCoreDirLister was simple). Please write TODO KF6 remove, I see zero reason to keep this a virtual method, and lxr says nobody is using it. Same for doMimeFilter. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73605. ahmadsamir added a comment. Add TODO KF6 to doNameFilter() REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73530=73605 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73530. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Verbatim REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73529=73530 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir added inline comments. INLINE COMMENTS > broulik wrote in kcoredirlister.h:595 > This class is exported, you cannot change this existing method and you also > cannot add a new `virtual`, see > https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Thanks, fixed. Luckily it's a convenience function of sorts and replacing the one instance it was used in KCoreDirLister was simple). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73529. ahmadsamir removed a subscriber: broulik. ahmadsamir added a comment. Verbatim REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73527=73529 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns, broulik
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir updated this revision to Diff 73527. ahmadsamir added a comment. Can't change doNameFilter due to BIC REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26659?vs=73515=73527 BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
broulik added inline comments. INLINE COMMENTS > kcoredirlister.h:595 > */ > -virtual bool doNameFilter(const QString , const QList > ) const; > +virtual bool doNameFilter(const QString , const > QVector ) const; > This class is exported, you cannot change this existing method and you also cannot add a new `virtual`, see https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26659 To: ahmadsamir, dfaure Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D26659: [KCoreDirLister] Port QRegExp to QRegularExpression
ahmadsamir created this revision. ahmadsamir added a reviewer: dfaure. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY doNameFilter() was the only protected function that took a QRegExp param., but I couldn't find any users of it in KDE. Also changed lstFilters from from QList to QVector. Port QRegExp::exactMatch() with QRegularExpression::anchoredPattern(), and QRegExp::Wildcard with QRegularExpression::wildcardToRegularExpression(). Note that setNameFilter() has some users, it took a QString param., which is used as the pattern for a QRegularExpression, but there are differences between valid QRegExp and QRegularExpression patterns. TEST PLAN make && ctest REPOSITORY R241 KIO BRANCH l-qregexp-deprecate (branched from master) REVISION DETAIL https://phabricator.kde.org/D26659 AFFECTED FILES src/core/kcoredirlister.cpp src/core/kcoredirlister.h src/core/kcoredirlister_p.h To: ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
