From 507428696747bd73a5b1842712e337f44569a7a8 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 18 Dec 2022 13:01:50 +0100 Subject: [PATCH 1/2] Refactor CompletionModel Do some clang-tidy cleanup --- src/common/CompletionModel.cpp | 35 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/common/CompletionModel.cpp b/src/common/CompletionModel.cpp index 3fc3fb9fb87..57d1eb1c86a 100644 --- a/src/common/CompletionModel.cpp +++ b/src/common/CompletionModel.cpp @@ -2,10 +2,8 @@ #include "Application.hpp" #include "common/ChatterSet.hpp" -#include "common/Common.hpp" #include "controllers/accounts/AccountController.hpp" #include "controllers/commands/CommandController.hpp" -#include "debug/Benchmark.hpp" #include "providers/twitch/TwitchChannel.hpp" #include "providers/twitch/TwitchCommon.hpp" #include "providers/twitch/TwitchIrcServer.hpp" @@ -64,7 +62,7 @@ QVariant CompletionModel::data(const QModelIndex &index, int) const auto it = this->items_.begin(); std::advance(it, index.row()); - return QVariant(it->string); + return {it->string}; } int CompletionModel::rowCount(const QModelIndex &) const @@ -85,7 +83,7 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) } // Twitch channel - auto tc = dynamic_cast(&this->channel_); + auto *tc = dynamic_cast(&this->channel_); auto addString = [=](const QString &str, TaggedString::Type type) { // Special case for handling default Twitch commands @@ -132,7 +130,8 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) // Twitch Emotes available locally auto localEmoteData = account->accessLocalEmotes(); - if (tc && localEmoteData->find(tc->roomId()) != localEmoteData->end()) + if (tc != nullptr && + localEmoteData->find(tc->roomId()) != localEmoteData->end()) { for (const auto &emote : localEmoteData->at(tc->roomId())) { @@ -143,18 +142,19 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) } // 7TV Global - for (auto &emote : *getApp()->twitch->getSeventvEmotes().globalEmotes()) + for (const auto &emote : + *getApp()->twitch->getSeventvEmotes().globalEmotes()) { addString(emote.first.string, TaggedString::Type::SeventvGlobalEmote); } // Bttv Global - for (auto &emote : *getApp()->twitch->getBttvEmotes().emotes()) + for (const auto &emote : *getApp()->twitch->getBttvEmotes().emotes()) { addString(emote.first.string, TaggedString::Type::BTTVChannelEmote); } // Ffz Global - for (auto &emote : *getApp()->twitch->getFfzEmotes().emotes()) + for (const auto &emote : *getApp()->twitch->getFfzEmotes().emotes()) { addString(emote.first.string, TaggedString::Type::FFZChannelEmote); } @@ -163,7 +163,7 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) if (prefix.startsWith(":")) { const auto &emojiShortCodes = getApp()->emotes->emojis.shortCodes; - for (auto &m : emojiShortCodes) + for (const auto &m : emojiShortCodes) { addString(QString(":%1:").arg(m), TaggedString::Type::Emoji); } @@ -171,7 +171,7 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) // // Stuff below is available only in regular Twitch channels - if (!tc) + if (tc == nullptr) { return; } @@ -205,36 +205,37 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord) } // 7TV Channel - for (auto &emote : *tc->seventvEmotes()) + for (const auto &emote : *tc->seventvEmotes()) { addString(emote.first.string, TaggedString::Type::SeventvChannelEmote); } // Bttv Channel - for (auto &emote : *tc->bttvEmotes()) + for (const auto &emote : *tc->bttvEmotes()) { addString(emote.first.string, TaggedString::Type::BTTVGlobalEmote); } // Ffz Channel - for (auto &emote : *tc->ffzEmotes()) + for (const auto &emote : *tc->ffzEmotes()) { addString(emote.first.string, TaggedString::Type::BTTVGlobalEmote); } // Custom Chatterino commands - for (auto &command : getApp()->commands->items) + for (const auto &command : getApp()->commands->items) { addString(command.name, TaggedString::CustomCommand); } // Default Chatterino commands - for (auto &command : getApp()->commands->getDefaultChatterinoCommandList()) + for (const auto &command : + getApp()->commands->getDefaultChatterinoCommandList()) { addString(command, TaggedString::ChatterinoCommand); } // Default Twitch commands - for (auto &command : TWITCH_DEFAULT_COMMANDS) + for (const auto &command : TWITCH_DEFAULT_COMMANDS) { addString(command, TaggedString::TwitchCommand); } @@ -246,7 +247,9 @@ bool CompletionModel::compareStrings(const QString &a, const QString &b) // (fixes order of LuL and LUL) int k = QString::compare(a, b, Qt::CaseInsensitive); if (k == 0) + { return a > b; + } return k < 0; } From 9ce86a6f17ae11a1668050bc00cf084a0a369664 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 18 Dec 2022 13:13:02 +0100 Subject: [PATCH 2/2] Use a shared mutex for CompletionModel's items mutex --- src/common/CompletionModel.cpp | 23 +++++++++++++++-------- src/common/CompletionModel.hpp | 17 +++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/common/CompletionModel.cpp b/src/common/CompletionModel.cpp index 57d1eb1c86a..fc4bd404960 100644 --- a/src/common/CompletionModel.cpp +++ b/src/common/CompletionModel.cpp @@ -22,8 +22,8 @@ namespace chatterino { // TaggedString // -CompletionModel::TaggedString::TaggedString(const QString &_string, Type _type) - : string(_string) +CompletionModel::TaggedString::TaggedString(QString _string, Type _type) + : string(std::move(_string)) , type(_type) { } @@ -51,30 +51,37 @@ CompletionModel::CompletionModel(Channel &channel) { } -int CompletionModel::columnCount(const QModelIndex &) const +int CompletionModel::columnCount(const QModelIndex &parent) const { + (void)parent; // unused + return 1; } -QVariant CompletionModel::data(const QModelIndex &index, int) const +QVariant CompletionModel::data(const QModelIndex &index, int role) const { - std::lock_guard lock(this->itemsMutex_); + (void)role; // unused + + std::shared_lock lock(this->itemsMutex_); auto it = this->items_.begin(); std::advance(it, index.row()); return {it->string}; } -int CompletionModel::rowCount(const QModelIndex &) const +int CompletionModel::rowCount(const QModelIndex &parent) const { - std::lock_guard lock(this->itemsMutex_); + (void)parent; // unused + + std::shared_lock lock(this->itemsMutex_); return this->items_.size(); } void CompletionModel::refresh(const QString &prefix, bool isFirstWord) { - std::lock_guard guard(this->itemsMutex_); + std::unique_lock lock(this->itemsMutex_); + this->items_.clear(); if (prefix.length() < 2 || !this->channel_.isTwitchChannel()) diff --git a/src/common/CompletionModel.hpp b/src/common/CompletionModel.hpp index 0d80c4aa6c4..c2670c08ee4 100644 --- a/src/common/CompletionModel.hpp +++ b/src/common/CompletionModel.hpp @@ -3,8 +3,8 @@ #include #include -#include #include +#include namespace chatterino { @@ -36,29 +36,30 @@ class CompletionModel : public QAbstractListModel TwitchCommand, }; - TaggedString(const QString &string, Type type); + TaggedString(QString _string, Type type); bool isEmote() const; bool operator<(const TaggedString &that) const; - QString string; - Type type; + const QString string; + const Type type; }; public: CompletionModel(Channel &channel); - virtual int columnCount(const QModelIndex &) const override; - virtual QVariant data(const QModelIndex &index, int) const override; - virtual int rowCount(const QModelIndex &) const override; + int columnCount(const QModelIndex &parent) const override; + QVariant data(const QModelIndex &index, int role) const override; + int rowCount(const QModelIndex &parent) const override; void refresh(const QString &prefix, bool isFirstWord = false); static bool compareStrings(const QString &a, const QString &b); private: + mutable std::shared_mutex itemsMutex_; std::set items_; - mutable std::mutex itemsMutex_; + Channel &channel_; };