improving code quality

chatterino.pro

@@ -258,7 +258,6 @@ HEADERS  += \
     src/common/Common.hpp \
     src/common/CompletionModel.hpp \
     src/common/FlagsEnum.hpp \
-    src/common/LockedObject.hpp \
     src/common/MutexValue.hpp \
     src/common/NetworkCommon.hpp \
     src/common/NetworkData.hpp \
@@ -271,7 +270,7 @@ HEADERS  += \
     src/common/NullablePtr.hpp \
     src/common/Property.hpp \
     src/common/ProviderId.hpp \
-    src/common/SerializeCustom.hpp \
+    src/uril/RapidJsonSerializeQString.hpp \
     src/common/SignalVectorModel.hpp \
     src/common/Version.hpp \
     src/controllers/accounts/Account.hpp \

src/BrowserExtension.cpp

@@ -31,11 +31,9 @@ void runLoop(NativeMessagingClient &client)
         char size_c[4];
         std::cin.read(size_c, 4);
 
-        if (std::cin.eof()) {
+        if (std::cin.eof()) break;
-            break;
-        }
 
-        uint32_t size = *reinterpret_cast<uint32_t *>(size_c);
+        auto size = *reinterpret_cast<uint32_t *>(size_c);
 
 #if 0
     bool bigEndian = isBigEndian();
@@ -50,12 +48,12 @@ void runLoop(NativeMessagingClient &client)
         }
 #endif
 
-        std::unique_ptr<char[]> b(new char[size + 1]);
+        std::unique_ptr<char[]> buffer(new char[size + 1]);
-        std::cin.read(b.get(), size);
+        std::cin.read(buffer.get(), size);
-        *(b.get() + size) = '\0';
+        *(buffer.get() + size) = '\0';
 
         client.sendMessage(
-            QByteArray::fromRawData(b.get(), static_cast<int32_t>(size)));
+            QByteArray::fromRawData(buffer.get(), static_cast<int32_t>(size)));
     }
 }
 }  // namespace

src/RunGui.cpp

@@ -27,35 +27,33 @@ void installCustomPalette()
 {
     // borrowed from
     // https://stackoverflow.com/questions/15035767/is-the-qt-5-dark-fusion-theme-available-for-windows
-    QPalette darkPalette = qApp->palette();
+    auto dark = qApp->palette();
 
-    darkPalette.setColor(QPalette::Window, QColor(22, 22, 22));
+    dark.setColor(QPalette::Window, QColor(22, 22, 22));
-    darkPalette.setColor(QPalette::WindowText, Qt::white);
+    dark.setColor(QPalette::WindowText, Qt::white);
-    darkPalette.setColor(QPalette::Text, Qt::white);
+    dark.setColor(QPalette::Text, Qt::white);
-    darkPalette.setColor(QPalette::Disabled, QPalette::WindowText,
+    dark.setColor(QPalette::Disabled, QPalette::WindowText,
-                         QColor(127, 127, 127));
+                  QColor(127, 127, 127));
-    darkPalette.setColor(QPalette::Base, QColor("#333"));
+    dark.setColor(QPalette::Base, QColor("#333"));
-    darkPalette.setColor(QPalette::AlternateBase, QColor("#444"));
+    dark.setColor(QPalette::AlternateBase, QColor("#444"));
-    darkPalette.setColor(QPalette::ToolTipBase, Qt::white);
+    dark.setColor(QPalette::ToolTipBase, Qt::white);
-    darkPalette.setColor(QPalette::ToolTipText, Qt::white);
+    dark.setColor(QPalette::ToolTipText, Qt::white);
-    darkPalette.setColor(QPalette::Disabled, QPalette::Text,
+    dark.setColor(QPalette::Disabled, QPalette::Text, QColor(127, 127, 127));
-                         QColor(127, 127, 127));
+    dark.setColor(QPalette::Dark, QColor(35, 35, 35));
-    darkPalette.setColor(QPalette::Dark, QColor(35, 35, 35));
+    dark.setColor(QPalette::Shadow, QColor(20, 20, 20));
-    darkPalette.setColor(QPalette::Shadow, QColor(20, 20, 20));
+    dark.setColor(QPalette::Button, QColor(70, 70, 70));
-    darkPalette.setColor(QPalette::Button, QColor(70, 70, 70));
+    dark.setColor(QPalette::ButtonText, Qt::white);
-    darkPalette.setColor(QPalette::ButtonText, Qt::white);
+    dark.setColor(QPalette::Disabled, QPalette::ButtonText,
-    darkPalette.setColor(QPalette::Disabled, QPalette::ButtonText,
+                  QColor(127, 127, 127));
-                         QColor(127, 127, 127));
+    dark.setColor(QPalette::BrightText, Qt::red);
-    darkPalette.setColor(QPalette::BrightText, Qt::red);
+    dark.setColor(QPalette::Link, QColor(42, 130, 218));
-    darkPalette.setColor(QPalette::Link, QColor(42, 130, 218));
+    dark.setColor(QPalette::Highlight, QColor(42, 130, 218));
-    darkPalette.setColor(QPalette::Highlight, QColor(42, 130, 218));
+    dark.setColor(QPalette::Disabled, QPalette::Highlight, QColor(80, 80, 80));
-    darkPalette.setColor(QPalette::Disabled, QPalette::Highlight,
+    dark.setColor(QPalette::HighlightedText, Qt::white);
-                         QColor(80, 80, 80));
+    dark.setColor(QPalette::Disabled, QPalette::HighlightedText,
-    darkPalette.setColor(QPalette::HighlightedText, Qt::white);
+                  QColor(127, 127, 127));
-    darkPalette.setColor(QPalette::Disabled, QPalette::HighlightedText,
-                         QColor(127, 127, 127));
 
-    qApp->setPalette(darkPalette);
+    qApp->setPalette(dark);
 }
 
 void initQt()

src/common/Channel.cpp

@@ -18,6 +18,9 @@
 
 namespace chatterino {
 
+//
+// Channel
+//
 Channel::Channel(const QString &name, Type type)
     : completionModel(name)
     , name_(name)
@@ -67,8 +70,7 @@ void Channel::addMessage(MessagePtr message)
 
     const QString &username = message->loginName;
     if (!username.isEmpty()) {
-        // TODO: Add recent chatters display name. This should maybe be a
+        // TODO: Add recent chatters display name
-        // setting
         this->addRecentChatter(message);
     }
 
@@ -194,7 +196,6 @@ void Channel::replaceMessage(MessagePtr message, MessagePtr replacement)
 
 void Channel::addRecentChatter(const MessagePtr &message)
 {
-    // Do nothing by default
 }
 
 bool Channel::canSendMessage() const
@@ -232,9 +233,45 @@ void Channel::onConnected()
 {
 }
 
-std::weak_ptr<Channel> Channel::weak_from_this()
+//
+// Indirect channel
+//
+IndirectChannel::Data::Data(ChannelPtr _channel, Channel::Type _type)
+    : channel(_channel)
+    , type(_type)
 {
-    return std::weak_ptr<Channel>(this->shared_from_this());
+}
+
+IndirectChannel::IndirectChannel(ChannelPtr channel, Channel::Type type)
+    : data_(std::make_unique<Data>(channel, type))
+{
+}
+
+ChannelPtr IndirectChannel::get()
+{
+    return data_->channel;
+}
+
+void IndirectChannel::reset(ChannelPtr channel)
+{
+    assert(this->data_->type != Channel::Type::Direct);
+
+    this->data_->channel = channel;
+    this->data_->changed.invoke();
+}
+
+pajlada::Signals::NoArgSignal &IndirectChannel::getChannelChanged()
+{
+    return this->data_->changed;
+}
+
+Channel::Type IndirectChannel::getType()
+{
+    if (this->data_->type == Channel::Type::Direct) {
+        return this->get()->getType();
+    } else {
+        return this->data_->type;
+    }
 }
 
 }  // namespace chatterino

src/common/Channel.hpp

@@ -4,7 +4,6 @@
 #include "messages/Image.hpp"
 #include "messages/LimitedQueue.hpp"
 #include "messages/Message.hpp"
-#include "util/ConcurrentMap.hpp"
 
 #include <QString>
 #include <QTimer>
@@ -66,9 +65,6 @@ public:
 
     CompletionModel completionModel;
 
-    // pre c++17 polyfill
-    std::weak_ptr<Channel> weak_from_this();
-
 protected:
     virtual void onConnected();
 
@@ -88,47 +84,17 @@ class IndirectChannel
         Channel::Type type;
         pajlada::Signals::NoArgSignal changed;
 
-        Data() = delete;
+        Data(ChannelPtr channel, Channel::Type type);
-        Data(ChannelPtr _channel, Channel::Type _type)
-            : channel(_channel)
-            , type(_type)
-        {
-        }
     };
 
 public:
     IndirectChannel(ChannelPtr channel,
-                    Channel::Type type = Channel::Type::Direct)
+                    Channel::Type type = Channel::Type::Direct);
-        : data_(new Data(channel, type))
-    {
-    }
 
-    ChannelPtr get()
+    ChannelPtr get();
-    {
+    void reset(ChannelPtr channel);
-        return data_->channel;
+    pajlada::Signals::NoArgSignal &getChannelChanged();
-    }
+    Channel::Type getType();
-
-    void update(ChannelPtr ptr)
-    {
-        assert(this->data_->type != Channel::Type::Direct);
-
-        this->data_->channel = ptr;
-        this->data_->changed.invoke();
-    }
-
-    pajlada::Signals::NoArgSignal &getChannelChanged()
-    {
-        return this->data_->changed;
-    }
-
-    Channel::Type getType()
-    {
-        if (this->data_->type == Channel::Type::Direct) {
-            return this->get()->getType();
-        } else {
-            return this->data_->type;
-        }
-    }
 
 private:
     std::shared_ptr<Data> data_;

src/common/LinkParser.cpp

@@ -5,69 +5,63 @@
 #include <QString>
 #include <QTextStream>
 
+// ip 0.0.0.0 - 224.0.0.0
+#define IP                                       \
+    "(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])"     \
+    "(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}" \
+    "(?:\\.(?:[1-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))"
+#define PORT "(?::\\d{2,5})"
+#define WEB_CHAR1 "[_a-z\\x{00a1}-\\x{ffff}0-9]"
+#define WEB_CHAR2 "[a-z\\x{00a1}-\\x{ffff}0-9]"
+
+#define SPOTIFY_1 "(?:artist|album|track|user:[^:]+:playlist):[a-zA-Z0-9]+"
+#define SPOTIFY_2 "user:[^:]+"
+#define SPOTIFY_3 "search:(?:[-\\w$\\.+!*'(),]+|%[a-fA-F0-9]{2})+"
+#define SPOTIFY_PARAMS "(?:" SPOTIFY_1 "|" SPOTIFY_2 "|" SPOTIFY_3 ")"
+#define SPOTIFY_LINK "(?x-mi:(spotify:" SPOTIFY_PARAMS "))"
+
+#define WEB_PROTOCOL "(?:(?:https?|ftps?)://)?"
+#define WEB_USER "(?:\\S+(?::\\S*)?@)?"
+#define WEB_HOST "(?:(?:" WEB_CHAR1 "-*)*" WEB_CHAR2 "+)"
+#define WEB_DOMAIN "(?:\\.(?:" WEB_CHAR2 "-*)*" WEB_CHAR2 "+)*"
+#define WEB_TLD "(?:" + tldData + ")"
+#define WEB_RESOURCE_PATH "(?:[/?#]\\S*)"
+#define WEB_LINK                                                              \
+    WEB_PROTOCOL WEB_USER "(?:" IP "|" WEB_HOST WEB_DOMAIN "\\." WEB_TLD PORT \
+                          "?" WEB_RESOURCE_PATH "?)"
+
+#define LINK "^(?:" SPOTIFY_LINK "|" WEB_LINK ")$"
+
 namespace chatterino {
 
 LinkParser::LinkParser(const QString &unparsedString)
 {
     static QRegularExpression linkRegex = [] {
         static QRegularExpression newLineRegex("\r?\n");
-        QFile tldFile(":/tlds.txt");
+        QFile file(":/tlds.txt");
-        tldFile.open(QFile::ReadOnly);
+        file.open(QFile::ReadOnly);
+        QTextStream tlds(&file);
+        tlds.setCodec("UTF-8");
 
-        QTextStream t1(&tldFile);
+        // tldData gets injected into the LINK macro
-        t1.setCodec("UTF-8");
+        auto tldData = tlds.readAll().replace(newLineRegex, "|");
+        (void)tldData;
 
-        // Read the TLDs in and replace the newlines with pipes
+        return QRegularExpression(LINK,
-        QString tldData = t1.readAll().replace(newLineRegex, "|");
-
-        const QString hyperlinkRegExp =
-            "^"
-            // Identifier for spotify
-            "(?x-mi:(spotify:(?:"
-            "(?:artist|album|track|user:[^:]+:playlist):"
-            "[a-zA-Z0-9]+|user:[^:]+|search:"
-            "(?:[-\\w$\\.+!*'(),]+|%[a-fA-F0-9]{2})+)))"
-            // If nothing matches then just go on
-            "|"
-            "^"
-            // Identifier for http and ftp
-            "(?:(?:https?|ftps?)://)?"
-            // user:pass authentication
-            "(?:\\S+(?::\\S*)?@)?"
-            "(?:"
-            // IP address dotted notation octets
-            // excludes loopback network 0.0.0.0
-            // excludes reserved space >= 224.0.0.0
-            // excludes network & broacast addresses
-            // (first & last IP address of each class)
-            "(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])"
-            "(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}"
-            "(?:\\.(?:[1-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))"
-            "|"
-            // host name
-            "(?:(?:[_a-z\\x{00a1}-\\x{ffff}0-9]-*)*[a-z\\x{00a1}-\\x{ffff}0-9]+"
-            ")"
-            // domain name
-            "(?:\\.(?:[a-z\\x{00a1}-\\x{ffff}0-9]-*)*[a-z\\x{00a1}-\\x{ffff}0-"
-            "9]+)*"
-            // TLD identifier
-            //"(?:\\.(?:[a-z\\x{00a1}-\\x{ffff}]{2,}))"
-            "(?:[\\.](?:" +
-            tldData +
-            "))"
-            "\\.?"
-            ")"
-            // port number
-            "(?::\\d{2,5})?"
-            // resource path
-            "(?:[/?#]\\S*)?"
-            "$";
-
-        return QRegularExpression(hyperlinkRegExp,
                                   QRegularExpression::CaseInsensitiveOption);
     }();
 
     this->match_ = linkRegex.match(unparsedString);
 }
 
+bool LinkParser::hasMatch() const
+{
+    return this->match_.hasMatch();
+}
+
+QString LinkParser::getCaptured() const
+{
+    return this->match_.captured();
+}
+
 }  // namespace chatterino

src/common/LinkParser.hpp

@@ -10,15 +10,8 @@ class LinkParser
 public:
     explicit LinkParser(const QString &unparsedString);
 
-    bool hasMatch() const
+    bool hasMatch() const;
-    {
+    QString getCaptured() const;
-        return this->match_.hasMatch();
-    }
-
-    QString getCaptured() const
-    {
-        return this->match_.captured();
-    }
 
 private:
     QRegularExpressionMatch match_;

src/common/LockedObject.hpp

@@ -1,38 +0,0 @@
-#pragma once
-
-#include <mutex>
-
-namespace chatterino {
-
-template <typename Type>
-class LockedObject
-{
-public:
-    LockedObject &operator=(const LockedObject<Type> &other)
-    {
-        this->mutex_.lock();
-
-        this->data = other.getValue();
-
-        this->mutex_.unlock();
-
-        return *this;
-    }
-
-    LockedObject &operator=(const Type &other)
-    {
-        this->mutex_.lock();
-
-        this->data = other;
-
-        this->mutex_.unlock();
-
-        return *this;
-    }
-
-private:
-    Type value_;
-    std::mutex mutex_;
-};
-
-}  // namespace chatterino

src/common/NetworkManager.cpp

@@ -5,11 +5,11 @@
 namespace chatterino {
 
 QThread NetworkManager::workerThread;
-QNetworkAccessManager NetworkManager::NaM;
+QNetworkAccessManager NetworkManager::accessManager;
 
 void NetworkManager::init()
 {
-    NetworkManager::NaM.moveToThread(&NetworkManager::workerThread);
+    NetworkManager::accessManager.moveToThread(&NetworkManager::workerThread);
     NetworkManager::workerThread.start();
 }
 

src/common/NetworkManager.hpp

@@ -20,7 +20,7 @@ class NetworkManager : public QObject
 
 public:
     static QThread workerThread;
-    static QNetworkAccessManager NaM;
+    static QNetworkAccessManager accessManager;
 
     static void init();
     static void deinit();

src/common/NetworkRequest.cpp

@@ -186,14 +186,14 @@ void NetworkRequest::doRequest()
         auto reply = [&]() -> QNetworkReply * {
             switch (data->requestType_) {
                 case NetworkRequestType::Get:
-                    return NetworkManager::NaM.get(data->request_);
+                    return NetworkManager::accessManager.get(data->request_);
 
                 case NetworkRequestType::Put:
-                    return NetworkManager::NaM.put(data->request_,
+                    return NetworkManager::accessManager.put(data->request_,
                                                    data->payload_);
 
                 case NetworkRequestType::Delete:
-                    return NetworkManager::NaM.deleteResource(data->request_);
+                    return NetworkManager::accessManager.deleteResource(data->request_);
 
                 default:
                     return nullptr;

src/common/NetworkRequest.hpp

@@ -58,8 +58,6 @@ public:
     void execute();
 
 private:
-    // Returns true if the file was successfully loaded from cache
-    // Returns false if the cache file either didn't exist, or it contained
     // "invalid" data "invalid" is specified by the onSuccess callback
     Outcome tryLoadCachedFile();
 

src/common/Property.hpp

@@ -1,34 +0,0 @@
-#pragma once
-
-#include "boost/noncopyable.hpp"
-
-namespace chatterino {
-
-template <typename T>
-class Property final : boost::noncopyable
-{
-public:
-    Property()
-    {
-    }
-
-    Property(const T &value)
-        : value_(value)
-    {
-    }
-
-    T &operator=(const T &f)
-    {
-        return value_ = f;
-    }
-
-    operator T const &() const
-    {
-        return value_;
-    }
-
-protected:
-    T value_;
-};
-
-}  // namespace chatterino

src/controllers/highlights/HighlightBlacklistUser.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "common/SerializeCustom.hpp"
+#include "util/RapidJsonSerializeQString.hpp"
 #include "util/RapidjsonHelpers.hpp"
 
 #include <QRegularExpression>

src/controllers/highlights/HighlightPhrase.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "common/SerializeCustom.hpp"
+#include "util/RapidJsonSerializeQString.hpp"
 #include "util/RapidjsonHelpers.hpp"
 
 #include <QRegularExpression>

src/controllers/ignores/IgnorePhrase.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "common/SerializeCustom.hpp"
+#include "util/RapidJsonSerializeQString.hpp"
 #include "util/RapidjsonHelpers.hpp"
 
 #include <QRegularExpression>

src/providers/twitch/TwitchChannel.cpp

@@ -250,12 +250,12 @@ void TwitchChannel::addPartedUser(const QString &user)
 
 QString TwitchChannel::getRoomId() const
 {
-    return this->roomID_.get();
+    return *this->roomID_.access();
 }
 
 void TwitchChannel::setRoomId(const QString &id)
 {
-    this->roomID_.set(id);
+    (*this->roomID_.access()) = id;
     this->roomIdChanged.invoke();
     this->loadRecentMessages();
 }

src/providers/twitch/TwitchChannel.hpp

@@ -134,7 +134,7 @@ private:
     const QString popoutPlayerUrl_;
 
     bool mod_ = false;
-    MutexValue<QString> roomID_;
+    UniqueAccess<QString> roomID_;
 
     UniqueAccess<QStringList> joinedUsers_;
     bool joinedUsersMergeQueued_ = false;

src/singletons/NativeMessaging.cpp

@@ -200,7 +200,7 @@ void NativeMessagingServer::ReceiverThread::handleMessage(
         if (_type == "twitch") {
             postToThread([=] {
                 if (!name.isEmpty()) {
-                    app->twitch.server->watchingChannel.update(
+                    app->twitch.server->watchingChannel.reset(
                         app->twitch.server->getOrAddChannel(name));
                 }
 

src/singletons/Theme.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
-#include "common/SerializeCustom.hpp"
 #include "common/Singleton.hpp"
+#include "util/RapidJsonSerializeQString.hpp"
 
 #include <QBrush>
 #include <QColor>

src/util/RapidjsonHelpers.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "common/SerializeCustom.hpp"
+#include "util/RapidJsonSerializeQString.hpp"
 
 #include <rapidjson/document.h>
 #include <pajlada/settings/serialize.hpp>

src/widgets/splits/Split.hpp

@@ -2,11 +2,11 @@
 
 #include "common/Channel.hpp"
 #include "common/NullablePtr.hpp"
-#include "common/SerializeCustom.hpp"
 #include "messages/LimitedQueueSnapshot.hpp"
 #include "messages/MessageElement.hpp"
 #include "messages/layouts/MessageLayout.hpp"
 #include "messages/layouts/MessageLayoutElement.hpp"
+#include "uril/RapidJsonSerializeQString.hpp"
 #include "widgets/BaseWidget.hpp"
 #include "widgets/helper/ChannelView.hpp"
 #include "widgets/helper/EffectLabel.hpp"