D23457: Port regex search to QRegularExpression
noreply at phabricator.kde.org
Mon Aug 26 20:55:03 BST 2019
cullmann added a comment.
In D23457#519217 <https://phabricator.kde.org/D23457#519217>, @ahmadsamir wrote:
> Maybe they'll also see it as ktexteditor/kate using a regex engine that matches what the abundance of online pcre docs say, and how other editors that use pcre behave?
> IIUC, '\s' was workedaround so as not to match a newline so that the search pattern wouldn't be considered multiline (isMultiLine() function), which makes findAll and replaceAll slower as it took longer, v.s. just matching against each line separately.
Actually, it is not faster.
If you take a look at the code, for single line regex, it iterates over the individual lines.
For multi line regexes, it will first concatinate all lines into one buffer.
For large files that is "very" slow.
And if you e.g. search + hit then "next match", this will be done again and again.
But given it only happens more often for stuff containing \s, I assume that should be not that problematic, thought not sure if the behavior change is that good.
> The thing is, what kateregexp did was replace '\s' with '[ \t]', which users who want this behaviour can easily use.
That is true, perhaps we should add this as extra into the menu as proposal, like \s/...
> Technically it's a whole new class, QRegularExpression, some different behaviours are sort of expected...
;=) That is really no good reasoning why one changes a behavior.
It is clear that if you port something over to a new class, behavior might change, but that doesn't make it a good thing per default.
On the other side, I see you did a lot of testing, that is highly appreciated.
I will think a bit more about this patch.
As you seems to have now played a bit with this part of the code, are you interested in test out the still not merged https://phabricator.kde.org/D19367 change?
To: ahmadsamir, #ktexteditor, dhaumann, cullmann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KWrite-Devel