D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

Ahmad Samir noreply at phabricator.kde.org
Tue Jan 7 07:11:22 GMT 2020


ahmadsamir added a comment.


  In D26366#589229 <https://phabricator.kde.org/D26366#589229>, @dfaure wrote:
  
  > So, the old way was 76 times faster than the new regexp :-)
  >
  > I'm not surprised, though, it's consistent with my experience with regexps.
  >
  > This might be a good reason to use the manual-search way. Especially now that you tested it for both performance and correctness :-)
  
  
  Less than one msec either way; so yeah, faster if one msec is even perceivable at all. :)
  
  > I'm also wondering if your regexp is completely correct. `:?(\w*)/?(\w*)` doesn't enforce that one word must be after ':' and one word must be after '/', since the 4 things are optional in an unrelated manner. Admittedly because of \w I can't come up with a string that would be misparsed, so maybe I'm wrong about this. There are ways to express this more strictly in regexp language, but it will only make it more complicated and likely slower :-)
  
  Hmm, it must be one word as there are no spaces allowed, and "*" is matching greedily, it'll keep matching \w all the way if there no spaces. A problem I see is it'll match @info:/plaintext, which probably won't work as "format" probably requires both a "role" and a "cue".
  
  What's making it easier to match with a regex here is that the input is "controlled", @foo:bar/baz, and written by developers who have to check the docs for the available values, and results are always checked by translators who'll complain if they get mal-formatted translation contexts.
  
  Anyway, I'll revert it in a separate patch; one issue arises here though, it looks like "\s" was over-kill in the original regex, given that only a literal white-space is what's expected, 99% percent of the times, to exist in a "context", i.e. not \t, \n or \r, we can just use .indexOf(QLatin1Char(' ')). I'll create the diff and publish it for review shortly.

REPOSITORY
  R249 KI18n

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

To: ahmadsamir, #frameworks, ilic, dfaure, mlaurent, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200107/b4e8e32a/attachment.html>


More information about the Kde-frameworks-devel mailing list