D23457: Port regex search to QRegularExpression

[Ahmad Samir]

ahmadsamir added a comment.


  In D23457#519773 <https://phabricator.kde.org/D23457#519773>, @cullmann wrote:
  
  > 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.
  
  
  I was mainly talking about find/replaceAll operations; qregularexpression is quite fast, I dabbled with using a global match and doing a findAll in one go, that was fast, but the code got complicated quite fast too. As I found out, ktexteditor wants the matches fed back to it one by one, since it has to do a lot of other stuff: highlighting, replacing text, undo history, buffer stuff, moving ranges... etc.
  [..]
  
  >> 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/...
  
  There's only so many menu entries that can be added, new users will have to read the docs at some point, regex is a complicated minefield.
  
  >> 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.
  
  True. But I also meant, that would be a good time to introduce new behaviours, as long as they are sane and adhere more to pcre standard behaviour. pcre documentation is impressive and with probably many guides floating around the internet, deviating from what the documentation says is potentially more annoying/frustrating to users.
  
  [..]
  
  > 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?
  
  I'll see what I can do.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D23457

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...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190827/27e7cf4d/attachment.html>