refactor: decouple reply parsing from MessageBuilder (#5660)

Chatterino · Oct 19, 2024 · 5c9b17c · 5c9b17c
1 parent 43bea0f
commit 5c9b17c
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -109,6 +109,7 @@
 - Dev: `GIFTimer` is no longer initialized in tests. (#5608)
 - Dev: Emojis now use flags instead of a set of strings for capabilities. (#5616)
 - Dev: Refactored static `MessageBuilder` helpers to standalone functions. (#5652)
+- Dev: Decoupled reply parsing from `MessageBuilder`. (#5660)
 
 ## 2.5.1
 

diff --git a/src/providers/twitch/IrcMessageHandler.cpp b/src/providers/twitch/IrcMessageHandler.cpp
@@ -123,24 +123,21 @@ int stripLeadingReplyMention(const QVariantMap &tags, QString &content)
     return 0;
 }
 
-void updateReplyParticipatedStatus(const QVariantMap &tags,
-                                   const QString &senderLogin,
-                                   MessageBuilder &builder,
-                                   std::shared_ptr<MessageThread> &thread,
-                                   bool isNew)
+[[nodiscard]] bool shouldHighlightReplyThread(
+    const QVariantMap &tags, const QString &senderLogin,
+    std::shared_ptr<MessageThread> &thread, bool isNew)
 {
     const auto &currentLogin =
         getApp()->getAccounts()->twitch.getCurrent()->getUserName();
 
     if (thread->subscribed())
     {
-        builder.message().flags.set(MessageFlag::SubscribedThread);
-        return;
+        return true;
     }
 
     if (thread->unsubscribed())
     {
-        return;
+        return false;
     }
 
     if (getSettings()->autoSubToParticipatedThreads)
@@ -154,8 +151,7 @@ void updateReplyParticipatedStatus(const QVariantMap &tags,
                 if (name == currentLogin)
                 {
                     thread->markSubscribed();
-                    builder.message().flags.set(MessageFlag::SubscribedThread);
-                    return;  // already marked as participated
+                    return true;  // already marked as participated
                 }
             }
         }
@@ -166,6 +162,8 @@ void updateReplyParticipatedStatus(const QVariantMap &tags,
             // don't set the highlight here
         }
     }
+
+    return false;
 }
 
 ChannelPtr channelOrEmptyByTarget(const QString &target,
@@ -242,10 +240,18 @@ QMap<QString, QString> parseBadges(const QString &badgesString)
     return badges;
 }
 
-void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
-                   const std::vector<MessagePtr> &otherLoaded,
-                   MessageBuilder &builder)
+struct ReplyContext {
+    std::shared_ptr<MessageThread> thread;
+    MessagePtr parent;
+    bool highlight = false;
+};
+
+[[nodiscard]] ReplyContext getReplyContext(
+    TwitchChannel *channel, Communi::IrcMessage *message,
+    const std::vector<MessagePtr> &otherLoaded)
 {
+    ReplyContext ctx;
+
     const auto &tags = message->tags();
     if (const auto it = tags.find("reply-thread-parent-msg-id");
         it != tags.end())
@@ -259,9 +265,9 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
             if (owned)
             {
                 // Thread already exists (has a reply)
-                updateReplyParticipatedStatus(tags, message->nick(), builder,
-                                              owned, false);
-                builder.setThread(owned);
+                ctx.highlight = shouldHighlightReplyThread(
+                    tags, message->nick(), owned, false);
+                ctx.thread = owned;
                 rootThread = owned;
             }
         }
@@ -295,10 +301,10 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
             {
                 std::shared_ptr<MessageThread> newThread =
                     std::make_shared<MessageThread>(foundMessage);
-                updateReplyParticipatedStatus(tags, message->nick(), builder,
-                                              newThread, true);
+                ctx.highlight = shouldHighlightReplyThread(
+                    tags, message->nick(), newThread, true);
 
-                builder.setThread(newThread);
+                ctx.thread = newThread;
                 rootThread = newThread;
                 // Store weak reference to thread in channel
                 channel->addReplyThread(newThread);
@@ -313,7 +319,7 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
             {
                 if (rootThread)
                 {
-                    builder.setParent(rootThread->root());
+                    ctx.parent = rootThread->root();
                 }
             }
             else
@@ -324,20 +330,22 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
                     auto thread = parentThreadIt->second.lock();
                     if (thread)
                     {
-                        builder.setParent(thread->root());
+                        ctx.parent = thread->root();
                     }
                 }
                 else
                 {
                     auto parent = channel->findMessage(parentID);
                     if (parent)
                     {
-                        builder.setParent(parent);
+                        ctx.parent = parent;
                     }
                 }
             }
         }
     }
+
+    return ctx;
 }
 
 std::optional<ClearChatMessage> parseClearChatMessage(
@@ -705,7 +713,13 @@ std::vector<MessagePtr> IrcMessageHandler::parseMessageWithReply(
                                privMsg->isAction());
         builder.setMessageOffset(messageOffset);
 
-        populateReply(tc, message, otherLoaded, builder);
+        auto replyCtx = getReplyContext(tc, message, otherLoaded);
+        builder.setThread(std::move(replyCtx.thread));
+        builder.setParent(std::move(replyCtx.parent));
+        if (replyCtx.highlight)
+        {
+            builder.message().flags.set(MessageFlag::SubscribedThread);
+        }
 
         if (!builder.isIgnored())
         {
@@ -1522,8 +1536,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
     QString content = originalContent;
     int messageOffset = stripLeadingReplyMention(tags, content);
 
-    MessageBuilder builder(channel, message, args, content, isAction);
-    builder.setMessageOffset(messageOffset);
+    ReplyContext replyCtx;
 
     if (const auto it = tags.find("reply-thread-parent-msg-id");
         it != tags.end())
@@ -1535,9 +1548,9 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
         {
             // Thread already exists (has a reply)
             auto thread = threadIt->second.lock();
-            updateReplyParticipatedStatus(tags, message->nick(), builder,
-                                          thread, false);
-            builder.setThread(thread);
+            replyCtx.highlight = shouldHighlightReplyThread(
+                tags, message->nick(), thread, false);
+            replyCtx.thread = thread;
             rootThread = thread;
         }
         else
@@ -1548,10 +1561,10 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
             {
                 // Found root reply message
                 auto newThread = std::make_shared<MessageThread>(root);
-                updateReplyParticipatedStatus(tags, message->nick(), builder,
-                                              newThread, true);
+                replyCtx.highlight = shouldHighlightReplyThread(
+                    tags, message->nick(), newThread, true);
 
-                builder.setThread(newThread);
+                replyCtx.thread = newThread;
                 rootThread = newThread;
                 // Store weak reference to thread in channel
                 channel->addReplyThread(newThread);
@@ -1566,7 +1579,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
             {
                 if (rootThread)
                 {
-                    builder.setParent(rootThread->root());
+                    replyCtx.parent = rootThread->root();
                 }
             }
             else
@@ -1577,21 +1590,31 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
                     auto thread = parentThreadIt->second.lock();
                     if (thread)
                     {
-                        builder.setParent(thread->root());
+                        replyCtx.parent = thread->root();
                     }
                 }
                 else
                 {
                     auto parent = channel->findMessage(parentID);
                     if (parent)
                     {
-                        builder.setParent(parent);
+                        replyCtx.parent = parent;
                     }
                 }
             }
         }
     }
 
+    MessageBuilder builder(channel, message, args, content, isAction);
+    builder.setMessageOffset(messageOffset);
+
+    builder.setThread(std::move(replyCtx.thread));
+    builder.setParent(std::move(replyCtx.parent));
+    if (replyCtx.highlight)
+    {
+        builder.message().flags.set(MessageFlag::SubscribedThread);
+    }
+
     if (isSub || !builder.isIgnored())
     {
         if (isSub)