client: Port old HighlightRule to ExpressionMatch
authorShane Synan <digitalcircuit36939@gmail.com>
Sat, 1 Sep 2018 21:29:02 +0000 (16:29 -0500)
committerManuel Nickschas <sputnick@quassel-irc.org>
Mon, 3 Sep 2018 20:12:02 +0000 (22:12 +0200)
Port QtUiMessageProcessor HighlightRule objects to ExpressionMatch
class, providing easy caching and simplifying expression handling.

Migrate HighlightRule struct into a full-blown class for easier
management and greater assurance over automatic internal cache
management.

Port QtUiMessageProcessor to ExpressionMatch for nickname matching,
providing easy caching and simplifying expression handling.

Add tons of documentation comments, too, and fix up line lengths.

NOTE: Legacy highlight rules lack support for "sender", "ID", and
inverse rules.  There's no technical reason for this, just lack of
developer time.  Feel free to add support in the future!  Just make
sure to not miss any part - I'd suggest simply replacing the legacy
LegacyHighlightRule class in QtUiMessageProcessor with the full one
from HighlightRuleManager, and don't forget to fix
> QtUiMessageProcessor::HighlightRule::operator!=()
...and...
> QtUiMessageProcessor::HighlightRule::determineExpressions()

src/qtui/qtuimessageprocessor.cpp
src/qtui/qtuimessageprocessor.h

index 0e0dca5..f1eb47f 100644 (file)
@@ -117,47 +117,62 @@ void QtUiMessageProcessor::checkForHighlight(Message &msg)
     // TODO: Cache this (per network)
     const Network *net = Client::network(msg.bufferInfo().networkId());
     if (net && !net->myNick().isEmpty()) {
     // TODO: Cache this (per network)
     const Network *net = Client::network(msg.bufferInfo().networkId());
     if (net && !net->myNick().isEmpty()) {
-        QStringList nickList;
-        if (_highlightNick == NotificationSettings::CurrentNick) {
-            nickList << net->myNick();
-        }
-        else if (_highlightNick == NotificationSettings::AllNicks) {
-            const Identity *myIdentity = Client::identity(net->identity());
-            if (myIdentity)
-                nickList = myIdentity->nicks();
-            if (!nickList.contains(net->myNick()))
-                nickList.prepend(net->myNick());
-        }
-        foreach(QString nickname, nickList) {
-            QRegExp nickRegExp("(^|\\W)" + QRegExp::escape(nickname) + "(\\W|$)", _nicksCaseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive);
-            if (nickRegExp.indexIn(stripFormatCodes(msg.contents())) >= 0) {
-                msg.setFlags(msg.flags() | Message::Highlight);
-                return;
-            }
+        // Get current nick
+        QString currentNick = net->myNick();
+        // Get identity nicks
+        QStringList identityNicks = {};
+        const Identity *myIdentity = Client::identity(net->identity());
+        if (myIdentity) {
+            identityNicks = myIdentity->nicks();
         }
 
         }
 
-        for (int i = 0; i < _highlightRules.count(); i++) {
-            const HighlightRule &rule = _highlightRules.at(i);
-            if (!rule.isEnabled)
+        // Get buffer name, message contents
+        QString bufferName = msg.bufferInfo().bufferName();
+        QString msgContents = msg.contents();
+        bool matches = false;
+
+        for (int i = 0; i < _highlightRuleList.count(); i++) {
+            auto &rule = _highlightRuleList.at(i);
+            if (!rule.isEnabled())
                 continue;
 
                 continue;
 
-            if (!rule.chanName.isEmpty()
-                    && !scopeMatch(msg.bufferInfo().bufferName(), rule.chanName,
-                                   rule.isRegExp, rule.caseSensitive)) {
+            // Skip if channel name doesn't match and channel rule is not empty
+            //
+            // Match succeeds if...
+            //   Channel name matches a defined rule
+            //   Defined rule is empty
+            // And take the inverse of the above
+            if (!rule.chanNameMatcher().match(bufferName, true)) {
                 // A channel name rule is specified and does NOT match the current buffer name, skip
                 // this rule
                 continue;
             }
 
                 // A channel name rule is specified and does NOT match the current buffer name, skip
                 // this rule
                 continue;
             }
 
-            QRegExp rx;
-            if (rule.isRegExp) {
-                rx = QRegExp(rule.name, rule.caseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive);
-            }
-            else {
-                rx = QRegExp("(^|\\W)" + QRegExp::escape(rule.name) + "(\\W|$)", rule.caseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive);
+            // Check message according to specified rule, allowing empty rules to match
+            bool contentsMatch = rule.contentsMatcher().match(stripFormatCodes(msgContents), true);
+
+            // Support for sender matching can be added here
+
+            if (contentsMatch) {
+                // Support for inverse rules can be added here
+                matches = true;
             }
             }
-            bool match = (rx.indexIn(stripFormatCodes(msg.contents())) >= 0);
-            if (match) {
+        }
+
+        if (matches) {
+            msg.setFlags(msg.flags() | Message::Highlight);
+            return;
+        }
+
+        // Check nicknames
+        if (_highlightNick != HighlightNickType::NoNick && !currentNick.isEmpty()) {
+            // Update cache if needed
+            determineNickExpressions(currentNick, identityNicks);
+
+            // Check for a match
+            if (_cachedNickMatcher.isValid()
+                    && _cachedNickMatcher.match(stripFormatCodes(msgContents))) {
+                // Nick matcher is valid and match found
                 msg.setFlags(msg.flags() | Message::Highlight);
                 return;
             }
                 msg.setFlags(msg.flags() | Message::Highlight);
                 return;
             }
@@ -169,6 +184,7 @@ void QtUiMessageProcessor::checkForHighlight(Message &msg)
 void QtUiMessageProcessor::nicksCaseSensitiveChanged(const QVariant &variant)
 {
     _nicksCaseSensitive = variant.toBool();
 void QtUiMessageProcessor::nicksCaseSensitiveChanged(const QVariant &variant)
 {
     _nicksCaseSensitive = variant.toBool();
+    _cacheNickConfigInvalid = true;
 }
 
 
 }
 
 
@@ -176,15 +192,15 @@ void QtUiMessageProcessor::highlightListChanged(const QVariant &variant)
 {
     QVariantList varList = variant.toList();
 
 {
     QVariantList varList = variant.toList();
 
-    _highlightRules.clear();
+    _highlightRuleList.clear();
     QVariantList::const_iterator iter = varList.constBegin();
     while (iter != varList.constEnd()) {
         QVariantMap rule = iter->toMap();
     QVariantList::const_iterator iter = varList.constBegin();
     while (iter != varList.constEnd()) {
         QVariantMap rule = iter->toMap();
-        _highlightRules << HighlightRule(rule["Name"].toString(),
-            rule["Enable"].toBool(),
-            rule["CS"].toBool() ? Qt::CaseSensitive : Qt::CaseInsensitive,
-            rule["RegEx"].toBool(),
-            rule["Channel"].toString());
+        _highlightRuleList << LegacyHighlightRule(rule["Name"].toString(),
+                rule["RegEx"].toBool(),
+                rule["CS"].toBool(),
+                rule["Enable"].toBool(),
+                rule["Channel"].toString());
         ++iter;
     }
 }
         ++iter;
     }
 }
@@ -193,4 +209,80 @@ void QtUiMessageProcessor::highlightListChanged(const QVariant &variant)
 void QtUiMessageProcessor::highlightNickChanged(const QVariant &variant)
 {
     _highlightNick = (NotificationSettings::HighlightNickType)variant.toInt();
 void QtUiMessageProcessor::highlightNickChanged(const QVariant &variant)
 {
     _highlightNick = (NotificationSettings::HighlightNickType)variant.toInt();
+    _cacheNickConfigInvalid = true;
+}
+
+
+void QtUiMessageProcessor::determineNickExpressions(const QString &currentNick,
+                                                    const QStringList identityNicks) const
+{
+    // Don't do anything for no nicknames
+    if (_highlightNick == HighlightNickType::NoNick) {
+        return;
+    }
+
+    // Only update if needed (check nickname config, current nick, identity nicks for change)
+    if (!_cacheNickConfigInvalid
+          && _cachedNickCurrent == currentNick
+          && _cachedIdentityNicks == identityNicks) {
+        return;
+    }
+
+    // Add all nicknames
+    QStringList nickList;
+    if (_highlightNick == HighlightNickType::CurrentNick) {
+        nickList << currentNick;
+    }
+    else if (_highlightNick == HighlightNickType::AllNicks) {
+        nickList = identityNicks;
+        if (!nickList.contains(currentNick))
+            nickList.prepend(currentNick);
+    }
+
+    // Set up phrase matcher, joining with newlines
+    _cachedNickMatcher = ExpressionMatch(nickList.join("\n"),
+                                        ExpressionMatch::MatchMode::MatchMultiPhrase,
+                                        _nicksCaseSensitive);
+
+    _cacheNickConfigInvalid = false;
+    _cachedNickCurrent = currentNick;
+    _cachedIdentityNicks = identityNicks;
+}
+
+
+/**************************************************************************
+ * LegacyHighlightRule
+ *************************************************************************/
+bool QtUiMessageProcessor::LegacyHighlightRule::operator!=(const LegacyHighlightRule &other) const
+{
+    return (_contents != other._contents ||
+            _isRegEx != other._isRegEx ||
+            _isCaseSensitive != other._isCaseSensitive ||
+            _isEnabled != other._isEnabled ||
+            _chanName != other._chanName);
+    // Don't compare ExpressionMatch objects as they are created as needed from the above
+}
+
+
+void QtUiMessageProcessor::LegacyHighlightRule::determineExpressions() const
+{
+    // Don't update if not needed
+    if (!_cacheInvalid) {
+        return;
+    }
+
+    // Set up matching rules
+    // Message is either phrase or regex
+    ExpressionMatch::MatchMode contentsMode =
+            _isRegEx ? ExpressionMatch::MatchMode::MatchRegEx :
+                       ExpressionMatch::MatchMode::MatchPhrase;
+    // Sender (when added) and channel are either multiple wildcard entries or regex
+    ExpressionMatch::MatchMode scopeMode =
+            _isRegEx ? ExpressionMatch::MatchMode::MatchRegEx :
+                       ExpressionMatch::MatchMode::MatchMultiWildcard;
+
+    _contentsMatch = ExpressionMatch(_contents, contentsMode, _isCaseSensitive);
+    _chanNameMatch = ExpressionMatch(_chanName, scopeMode, _isCaseSensitive);
+
+    _cacheInvalid = false;
 }
 }
index 5f63af7..d8559d3 100644 (file)
@@ -24,6 +24,7 @@
 #include <QTimer>
 
 #include "abstractmessageprocessor.h"
 #include <QTimer>
 
 #include "abstractmessageprocessor.h"
+#include "expressionmatch.h"
 
 class QtUiMessageProcessor : public AbstractMessageProcessor
 {
 
 class QtUiMessageProcessor : public AbstractMessageProcessor
 {
@@ -53,28 +54,224 @@ private slots:
     void highlightNickChanged(const QVariant &variant);
 
 private:
     void highlightNickChanged(const QVariant &variant);
 
 private:
+    /**
+     * Individual highlight rule (legacy client-side version)
+     */
+    class LegacyHighlightRule
+    {
+    public:
+        /**
+         * Construct an empty highlight rule
+         */
+        LegacyHighlightRule() {}
+
+        /**
+         * Construct a highlight rule with the given parameters
+         *
+         * @param contents         String representing a message contents expression to match
+         * @param isRegEx          True if regular expression, otherwise false
+         * @param isCaseSensitive  True if case sensitive, otherwise false
+         * @param isEnabled        True if enabled, otherwise false
+         * @param chanName         String representing a channel name expression to match
+         */
+        LegacyHighlightRule(QString contents, bool isRegEx, bool isCaseSensitive, bool isEnabled,
+                      QString chanName)
+            : _contents(contents), _isRegEx(isRegEx), _isCaseSensitive(isCaseSensitive),
+              _isEnabled(isEnabled), _chanName(chanName)
+        {
+            _cacheInvalid = true;
+            // Cache expression matches on construction
+            //
+            // This provides immediate feedback on errors when loading the rule.  If profiling shows
+            // this as a performance bottleneck, this can be removed in deference to caching on
+            // first use.
+            //
+            // Inversely, if needed for validity checks, caching can be done on every update below
+            // instead of on first use.
+            determineExpressions();
+        }
+
+        /**
+         * Gets the message contents this rule matches
+         *
+         * NOTE: Use HighlightRule::contentsMatcher() for performing matches
+         *
+         * CAUTION: For legacy reasons, "contents" doubles as the identifier for the ignore rule.
+         * Duplicate entries are not allowed.
+         *
+         * @return String representing a phrase or expression to match
+         */
+        inline QString contents() const {
+            return _contents;
+        }
+        /**
+         * Sets the message contents this rule matches
+         *
+         * @param contents String representing a phrase or expression to match
+         */
+        inline void setContents(const QString &contents) {
+            _contents = contents;
+            _cacheInvalid = true;
+        }
+
+        /**
+         * Gets if this is a regular expression rule
+         *
+         * @return True if regular expression, otherwise false
+         */
+        inline bool isRegEx() const {
+            return _isRegEx;
+        }
+        /**
+         * Sets if this rule is a regular expression rule
+         *
+         * @param isRegEx True if regular expression, otherwise false
+         */
+        inline void setIsRegEx(bool isRegEx) {
+            _isRegEx = isRegEx;
+            _cacheInvalid = true;
+        }
+
+        /**
+         * Gets if this rule is case sensitive
+         *
+         * @return True if case sensitive, otherwise false
+         */
+        inline bool isCaseSensitive() const {
+            return _isCaseSensitive;
+        }
+        /**
+         * Sets if this rule is case sensitive
+         *
+         * @param isCaseSensitive True if case sensitive, otherwise false
+         */
+        inline void setIsCaseSensitive(bool isCaseSensitive) {
+            _isCaseSensitive = isCaseSensitive;
+            _cacheInvalid = true;
+        }
+
+        /**
+         * Gets if this rule is enabled and active
+         *
+         * @return True if enabled, otherwise false
+         */
+        inline bool isEnabled() const {
+            return _isEnabled;
+        }
+        /**
+         * Sets if this rule is enabled and active
+         *
+         * @param isEnabled True if enabled, otherwise false
+         */
+        inline void setIsEnabled(bool isEnabled) {
+            _isEnabled = isEnabled;
+        }
+
+        /**
+         * Gets the channel name this rule matches
+         *
+         * NOTE: Use HighlightRule::chanNameMatcher() for performing matches
+         *
+         * @return String representing a phrase or expression to match
+         */
+        inline QString chanName() const {
+            return _chanName;
+        }
+        /**
+         * Sets the channel name this rule matches
+         *
+         * @param chanName String representing a phrase or expression to match
+         */
+        inline void setChanName(const QString &chanName) {
+            _chanName = chanName;
+            _cacheInvalid = true;
+        }
+
+        /**
+         * Gets the expression matcher for the message contents, caching if needed
+         *
+         * @return Expression matcher to compare with message contents
+         */
+        inline ExpressionMatch contentsMatcher() const {
+            if (_cacheInvalid) {
+                determineExpressions();
+            }
+            return _contentsMatch;
+        }
+
+        /**
+         * Gets the expression matcher for the channel name, caching if needed
+         *
+         * @return Expression matcher to compare with channel name
+         */
+        inline ExpressionMatch chanNameMatcher() const {
+            if (_cacheInvalid) {
+                determineExpressions();
+            }
+            return _chanNameMatch;
+        }
+
+        bool operator!=(const LegacyHighlightRule &other) const;
+
+    private:
+        /**
+         * Update internal cache of expression matching if needed
+         */
+        void determineExpressions() const;
+
+        QString _contents = {};
+        bool _isRegEx = false;
+        bool _isCaseSensitive = false;
+        bool _isEnabled = true;
+        QString _chanName = {};
+
+        // These represent internal cache and should be safe to mutate in 'const' functions
+        // See https://stackoverflow.com/questions/3141087/what-is-meant-with-const-at-end-of-function-declaration
+        mutable bool _cacheInvalid = true;           ///< If true, match cache needs redone
+        mutable ExpressionMatch _contentsMatch = {}; ///< Expression match cache for message content
+        mutable ExpressionMatch _chanNameMatch = {}; ///< Expression match cache for channel name
+    };
+
+    using LegacyHighlightRuleList = QList<LegacyHighlightRule>;
+
     void checkForHighlight(Message &msg);
     void startProcessing();
 
     void checkForHighlight(Message &msg);
     void startProcessing();
 
+    using HighlightNickType = NotificationSettings::HighlightNickType;
+
+    /**
+     * Update internal cache of expression matching if needed
+     */
+    void determineNickExpressions(const QString &currentNick,
+                                  const QStringList identityNicks) const;
+
+    /**
+     * Check if nickname matching cache is invalid
+     * @param currentNick
+     * @param identityNicks
+     * @return
+     */
+    bool cacheNickInvalid(const QString &currentNick, const QStringList identityNicks) const {
+        if (_cacheNickConfigInvalid) return true;
+        if (_cachedNickCurrent != currentNick) return true;
+        if (_cachedIdentityNicks != identityNicks) return true;
+    }
+
+    LegacyHighlightRuleList _highlightRuleList;
+    HighlightNickType _highlightNick = HighlightNickType::CurrentNick;
+    bool _nicksCaseSensitive = false;
+
+    // These represent internal cache and should be safe to mutate in 'const' functions
+    mutable bool _cacheNickConfigInvalid = true;     ///< If true, nick match cache needs redone
+    mutable QString _cachedNickCurrent = {};         ///< Last cached current nick
+    mutable QStringList _cachedIdentityNicks = {};   ///< Last cached identity nicks
+    mutable ExpressionMatch _cachedNickMatcher = {}; ///< Expression match cache for nicks
+
     QList<QList<Message> > _processQueue;
     QList<Message> _currentBatch;
     QTimer _processTimer;
     bool _processing;
     Mode _processMode;
     QList<QList<Message> > _processQueue;
     QList<Message> _currentBatch;
     QTimer _processTimer;
     bool _processing;
     Mode _processMode;
-
-    struct HighlightRule {
-        QString name;
-        bool isEnabled;
-        Qt::CaseSensitivity caseSensitive;
-        bool isRegExp;
-        QString chanName;
-        inline HighlightRule(const QString &name, bool enabled, Qt::CaseSensitivity cs, bool regExp, const QString &chanName)
-            : name(name), isEnabled(enabled), caseSensitive(cs), isRegExp(regExp), chanName(chanName) {}
-    };
-
-    QList<HighlightRule> _highlightRules;
-    NotificationSettings::HighlightNickType _highlightNick;
-    bool _nicksCaseSensitive;
 };
 
 
 };