[KCoreDirLister] Port QRegExp to QRegularExpression
ClosedPublic

Authored by ahmadsamir on Jan 14 2020, 12:18 PM.

Details

Summary

Due to BIC we can't change doNameFilter(); since it was a convenience
function of sorts, just replace the one time it was actually used and
use a for loop directly to iterate over the regex patterns.

Add TODO KF6 notes to remove doNameFilter() and doMimeFilter(), two
virtual methods that have no users (c.f. the review).

Port QRegExp::exactMatch() with QRegularExpression::anchoredPattern(),
and QRegExp::Wildcard with QRegularExpression::wildcardToRegularExpression().
FTR wildcardToRegularExpression() returns an anchored pattern, therefore
there is no need to use anchoredPattern() with wildcardToRegularExpression().

Test Plan

make && ctest

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahmadsamir created this revision.Jan 14 2020, 12:18 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 14 2020, 12:18 PM
ahmadsamir requested review of this revision.Jan 14 2020, 12:18 PM
broulik added inline comments.
src/core/kcoredirlister.h
594

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

ahmadsamir updated this revision to Diff 73527.Jan 14 2020, 2:42 PM

Can't change doNameFilter due to BIC

ahmadsamir updated this revision to Diff 73529.Jan 14 2020, 2:44 PM
ahmadsamir removed a subscriber: broulik.

Verbatim

ahmadsamir added inline comments.Jan 14 2020, 2:44 PM
src/core/kcoredirlister.h
594

Thanks, fixed. Luckily it's a convenience function of sorts and replacing the one instance it was used in KCoreDirLister was simple).

ahmadsamir updated this revision to Diff 73530.Jan 14 2020, 2:47 PM
ahmadsamir edited the summary of this revision. (Show Details)

Verbatim

ahmadsamir updated this revision to Diff 73605.Jan 15 2020, 9:45 AM

Add TODO KF6 to doNameFilter()

dfaure requested changes to this revision.Jan 16 2020, 8:35 AM
dfaure added inline comments.
src/core/kcoredirlister.h
594

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.

This revision now requires changes to proceed.Jan 16 2020, 8:35 AM
ahmadsamir updated this revision to Diff 73676.Jan 16 2020, 8:48 AM
ahmadsamir edited the summary of this revision. (Show Details)

Add TODO KF6 notes to remove doNameFilter() and doMimeFilter()

dfaure requested changes to this revision.Jan 16 2020, 10:39 PM
dfaure added inline comments.
src/core/kcoredirlister.cpp
2306

I think you said this would anchor twice, in another review? Needs to be fixed then.

(Good for readability!)

This revision now requires changes to proceed.Jan 16 2020, 10:39 PM
ahmadsamir added inline comments.Jan 17 2020, 7:32 AM
src/core/kcoredirlister.cpp
2306

Right. (I searched through all the porting commits, I missed this one as it hasn't committed yet...).

ahmadsamir updated this revision to Diff 73796.Jan 17 2020, 9:12 PM
  • 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
dfaure accepted this revision.Jan 18 2020, 11:40 AM
This revision is now accepted and ready to land.Jan 18 2020, 11:40 AM
ahmadsamir updated this revision to Diff 73989.Jan 21 2020, 8:25 AM
ahmadsamir edited the summary of this revision. (Show Details)

Rebase and tweak the commit message

This revision was automatically updated to reflect the committed changes.

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<QRegularExpression>'
09:29:39    148 |         QVector<QRegularExpression> lstFilters;

I just needed to #include <QVector> to fix it

Fixed, thanks