| mlaurent | |
| dfaure |
| Frameworks |
Deprecate the KFind::find() and KReplace::replace() methods that take a
QRegExp; Regular Expression matches will be handled internally if the
KFind::RegularExpression flag is set.
Port QRegExp::setMinimal() by making the regex pattern non-greedy, where
possible.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
A couple of notes:
OK for deprecating Incremental. A full revert (at KF6 time) of commit 9ac82e27ad0322e444c16 looks tricky though :-)
I searched the git history, and I can't see where we ever used FindIncremental... [I searched kdelibs, since I'm pretty sure this was added for KHTML]
You say that nothing calls find(QRegExp), that's because the find dialog just sets the option RegularExpression, right?
This makes sense actually. I don't see why we have this API. The user uses the dialog and checks a box...
It sounds like we should just deprecate find(QRegExp) and point to the RegularExpression option.
(and make it private, as you said, i.e. move to private class)
| src/findreplace/kfind.cpp | ||
|---|---|---|
| 759–762 | typo: additional 'e' in regeExp | |
| src/findreplace/kfinddialog.cpp | ||
| 567 | I doubt end users want to open Qt API documentation... | |
It's got to be removed at KF6; since it's not actually used, it'll make the code simpler, I think (partial/incremental matching is usually a complex pain).
You say that nothing calls find(QRegExp), that's because the find dialog just sets the option RegularExpression, right?
This makes sense actually. I don't see why we have this API. The user uses the dialog and checks a box...
Yeah, I meant explicitly call Kfind::find() that takes a QRegExp.
It sounds like we should just deprecate find(QRegExp) and point to the RegularExpression option.
(and make it private, as you said, i.e. move to private class)
OK, will do.
| src/findreplace/kfinddialog.cpp | ||
|---|---|---|
| 567 | One alternative is https://perldoc.perl.org/perlre.html :) I could opt for: then one online search later, the user finds what PCRE is. | |
Looking at the incremental stuff some more, there are no public methods that mention incremental; the only way is to set the KFind::FindIncremental flag; the only method I could find is in the private API (find_p.h) startNewIncrementalSearch(). So technically it's all hidden behind the d-pointer, there's nothing to deprecate about incremental search, except remove the flag/enum FindIncremental... (does that mean we can remove the incremental code now, or wait till the KF6 branching?).
FWIW, I checked lxr again and KFind::FindIncremental exists only in kmplayer and khtml, and in both cases it's in commented out code.
Deprecate the attribute, keep the code. There is more KF5-based code out there than the one visible by lxr.kde.org. Some of it not even public. We make the same promise as Qt, preserving SC and BC in major versions.
OK for "Invalid PCRE pattern syntax"
CI doesn't like this commit.
FAIL! : TestKFind::testStaticFindRegexp(whole words ok) Compared values are not the same Actual (result2) : 1 Expected (expectedResult): 9 Loc: [/home/jenkins/workspace/Frameworks/ktextwidgets/kf5-qt5 SUSEQt5.14/autotests/kfindtest.cpp(242)]
Please check.