[KCoreDirLister] Port QRegExp to QRegularExpression
AcceptedPublic

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

Details

Reviewers
dfaure
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().

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

Diff Detail

Repository
R241 KIO
Branch
l-qregexp-deprecate (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21299
Build 21317: arc lint + arc unit
ahmadsamir created this revision.Tue, Jan 14, 12:18 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptTue, Jan 14, 12:18 PM
ahmadsamir requested review of this revision.Tue, Jan 14, 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.Tue, Jan 14, 2:42 PM

Can't change doNameFilter due to BIC

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

Verbatim

ahmadsamir added inline comments.Tue, Jan 14, 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.Tue, Jan 14, 2:47 PM
ahmadsamir edited the summary of this revision. (Show Details)

Verbatim

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

Add TODO KF6 to doNameFilter()

dfaure requested changes to this revision.Thu, Jan 16, 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.Thu, Jan 16, 8:35 AM
ahmadsamir updated this revision to Diff 73676.Thu, Jan 16, 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.Thu, Jan 16, 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.Thu, Jan 16, 10:39 PM
ahmadsamir added inline comments.Fri, Jan 17, 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.Fri, Jan 17, 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.Sat, Jan 18, 11:40 AM
This revision is now accepted and ready to land.Sat, Jan 18, 11:40 AM