From ee8b9f55860e340c1600188fddcfd557c7489f66 Mon Sep 17 00:00:00 2001 From: Shane Synan Date: Sat, 1 Sep 2018 16:36:45 -0500 Subject: [PATCH] common: Port remote nicks to NickHighlightMatcher Port HighlightRuleManager nick highlights to NickHighlightMatcher class, providing easy caching and simplifying expression handling. This fixes nickname caching being reset when switching between networks. Add SIGNAL/SLOT traversal to pass on information about network removal to clean up per-network nickname highlight caches, avoiding memory leaks. --- src/client/client.cpp | 3 ++ src/common/highlightrulemanager.cpp | 55 ++++----------------------- src/common/highlightrulemanager.h | 55 +++++++++++++-------------- src/core/corehighlightrulemanager.cpp | 3 +- src/core/coresession.cpp | 3 ++ 5 files changed, 42 insertions(+), 77 deletions(-) diff --git a/src/client/client.cpp b/src/client/client.cpp index 23b838ff..c20fa8b6 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -444,6 +444,9 @@ void Client::setSyncedToCore() Q_ASSERT(!_highlightRuleManager); _highlightRuleManager = new HighlightRuleManager(this); p->synchronize(highlightRuleManager()); + // Listen to network removed events + connect(this, SIGNAL(networkRemoved(NetworkId)), + _highlightRuleManager, SLOT(networkRemoved(NetworkId))); /* not ready yet // create TransferManager and DccConfig if core supports them diff --git a/src/common/highlightrulemanager.cpp b/src/common/highlightrulemanager.cpp index c9c4cfcd..d3c23846 100644 --- a/src/common/highlightrulemanager.cpp +++ b/src/common/highlightrulemanager.cpp @@ -141,13 +141,14 @@ void HighlightRuleManager::addHighlightRule(int id, const QString &name, bool is } -bool HighlightRuleManager::match(const QString &msgContents, +bool HighlightRuleManager::match(const NetworkId &netId, + const QString &msgContents, const QString &msgSender, Message::Type msgType, Message::Flags msgFlags, const QString &bufferName, const QString ¤tNick, - const QStringList identityNicks) + const QStringList &identityNicks) { if (!((msgType & (Message::Plain | Message::Notice | Message::Action)) && !(msgFlags & Message::Self))) { return false; @@ -194,13 +195,9 @@ bool HighlightRuleManager::match(const QString &msgContents, // 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 + // Nickname matching allowed and current nickname is known + // Run the nickname matcher on the unformatted string + if (_nickMatcher.match(stripFormatCodes(msgContents), netId, currentNick, identityNicks)) { return true; } } @@ -228,44 +225,8 @@ void HighlightRuleManager::toggleHighlightRule(int highlightRule) bool HighlightRuleManager::match(const Message &msg, const QString ¤tNick, const QStringList &identityNicks) { - return match(msg.contents(), msg.sender(), msg.type(), msg.flags(), msg.bufferInfo().bufferName(), currentNick, identityNicks); -} - - -void HighlightRuleManager::determineNickExpressions(const QString ¤tNick, - 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 == CurrentNick) { - nickList << currentNick; - } - else if (_highlightNick == 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; + return match(msg.bufferInfo().networkId(), msg.contents(), msg.sender(), msg.type(), msg.flags(), + msg.bufferInfo().bufferName(), currentNick, identityNicks); } diff --git a/src/common/highlightrulemanager.h b/src/common/highlightrulemanager.h index cabf0d25..d98c8675 100644 --- a/src/common/highlightrulemanager.h +++ b/src/common/highlightrulemanager.h @@ -29,6 +29,7 @@ #include "expressionmatch.h" #include "message.h" +#include "nickhighlightmatcher.h" #include "syncableobject.h" class HighlightRuleManager : public SyncableObject @@ -364,7 +365,10 @@ public slots: inline void setHighlightNick(int highlightNick) { _highlightNick = static_cast(highlightNick); - _cacheNickConfigInvalid = true; + // Convert from HighlightRuleManager::HighlightNickType to + // NickHighlightMatcher::HighlightNickType + _nickMatcher.setHighlightMode( + static_cast(_highlightNick)); } virtual inline void requestSetNicksCaseSensitive(bool nicksCaseSensitive) @@ -374,49 +378,42 @@ public slots: inline void setNicksCaseSensitive(bool nicksCaseSensitive) { _nicksCaseSensitive = nicksCaseSensitive; - _cacheNickConfigInvalid = true; + // Update nickname matcher, too + _nickMatcher.setCaseSensitive(nicksCaseSensitive); + } + + /** + * Network removed from system + * + * Handles cleaning up cache from stale networks. + * + * @param id Network ID of removed network + */ + inline void networkRemoved(NetworkId id) { + // Clean up nickname matching cache + _nickMatcher.removeNetwork(id); } protected: void setHighlightRuleList(const QList &HighlightRuleList) { _highlightRuleList = HighlightRuleList; } - bool match(const QString &msgContents, + bool match(const NetworkId &netId, + const QString &msgContents, const QString &msgSender, Message::Type msgType, Message::Flags msgFlags, const QString &bufferName, const QString ¤tNick, - const QStringList identityNicks); + const QStringList &identityNicks); signals: void ruleAdded(QString name, bool isRegEx, bool isCaseSensitive, bool isEnabled, bool isInverse, QString sender, QString chanName); private: - /** - * Update internal cache of expression matching if needed - */ - void determineNickExpressions(const QString ¤tNick, - const QStringList identityNicks) const; + HighlightRuleList _highlightRuleList = {}; ///< Custom highlight rule list + NickHighlightMatcher _nickMatcher = {}; ///< Nickname highlight matcher - /** - * Check if nickname matching cache is invalid - * @param currentNick - * @param identityNicks - * @return - */ - bool cacheNickInvalid(const QString ¤tNick, const QStringList identityNicks) const { - if (_cacheNickConfigInvalid) return true; - if (_cachedNickCurrent != currentNick) return true; - if (_cachedIdentityNicks != identityNicks) return true; - } - - HighlightRuleList _highlightRuleList; + /// Nickname highlighting mode 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 + bool _nicksCaseSensitive = false; ///< If true, match nicknames with exact case }; diff --git a/src/core/corehighlightrulemanager.cpp b/src/core/corehighlightrulemanager.cpp index eed5655b..b9c81bea 100644 --- a/src/core/corehighlightrulemanager.cpp +++ b/src/core/corehighlightrulemanager.cpp @@ -48,5 +48,6 @@ void CoreHighlightRuleManager::save() bool CoreHighlightRuleManager::match(const RawMessage &msg, const QString ¤tNick, const QStringList &identityNicks) { - return match(msg.text, msg.sender, msg.type, msg.flags, msg.target, currentNick, identityNicks); + return match(msg.networkId, msg.text, msg.sender, msg.type, msg.flags, msg.target, currentNick, + identityNicks); } diff --git a/src/core/coresession.cpp b/src/core/coresession.cpp index 200fadcc..359c3658 100644 --- a/src/core/coresession.cpp +++ b/src/core/coresession.cpp @@ -142,6 +142,9 @@ CoreSession::CoreSession(UserId uid, bool restoreState, bool strictIdentEnabled, p->synchronize(_coreInfo); p->synchronize(&_ignoreListManager); p->synchronize(&_highlightRuleManager); + // Listen to network removed events + connect(this, SIGNAL(networkRemoved(NetworkId)), + &_highlightRuleManager, SLOT(networkRemoved(NetworkId))); p->synchronize(transferManager()); // Restore session state if (restoreState) -- 2.20.1