dhaumann added a comment.
I wonder if the `?:` optimizations make sense. QRegularExpression has the
option `QRegularExpression::DontCaptureOption` to not capture anything. Looking
into our code we have:
561 bool RegExpr::doLoad(QXmlStreamReader& reader)
562 {
563
m_regexp.setPattern(reader.attributes().value(QStringLiteral("String")).toString());
// here we set the pattern -> OK
564
565 const auto isMinimal =
Xml::attrToBool(reader.attributes().value(QStringLiteral("minimal")));
566 const auto isCaseInsensitive =
Xml::attrToBool(reader.attributes().value(QStringLiteral("insensitive")));
567 m_regexp.setPatternOptions(
// if (m_dynamic == false), we could add
the
568 (isMinimal ? QRegularExpression::InvertedGreedinessOption :
QRegularExpression::NoPatternOption) | // flag
QRegularExpression::DontCaptureOption
569 (isCaseInsensitive ? QRegularExpression::CaseInsensitiveOption :
QRegularExpression::NoPatternOption));
570
571 // optimize the pattern for the non-dynamic case, we use them OFTEN
572 m_dynamic =
Xml::attrToBool(reader.attributes().value(QStringLiteral("dynamic")));
573 if (!m_dynamic) {
574 m_regexp.optimize();
575 }
576 // [...]
In other words: The current patch adds many `?:` which also makes the RegExps
harder to read. So: Do we really get a performance gain here? Wouldn't it be
possible to get an even better performance gain by using the flag
`DontCaptureOption`?
Currently, I am not yet convinced, can you give this a try? :-)
REPOSITORY
R216 Syntax Highlighting
REVISION DETAIL
https://phabricator.kde.org/D24982
To: nibags, #framework_syntax_highlighting, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh,
ngraham, bruns, demsking, cullmann, sars, dhaumann
