From 5c9b17c31a02d6fa69f61ae96a2258fedd6cb798 Mon Sep 17 00:00:00 2001 From: nerix Date: Sat, 19 Oct 2024 14:04:44 +0200 Subject: [PATCH] refactor: decouple reply parsing from MessageBuilder (#5660) --- CHANGELOG.md | 1 + src/providers/twitch/IrcMessageHandler.cpp | 91 ++++++++++++++-------- 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7212d43..80d38c0fb 100644 --- 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 index 9724ec213..7d5bb34a4 100644 --- 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 &thread, - bool isNew) +[[nodiscard]] bool shouldHighlightReplyThread( + const QVariantMap &tags, const QString &senderLogin, + std::shared_ptr &thread, bool isNew) { const auto ¤tLogin = 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 parseBadges(const QString &badgesString) return badges; } -void populateReply(TwitchChannel *channel, Communi::IrcMessage *message, - const std::vector &otherLoaded, - MessageBuilder &builder) +struct ReplyContext { + std::shared_ptr thread; + MessagePtr parent; + bool highlight = false; +}; + +[[nodiscard]] ReplyContext getReplyContext( + TwitchChannel *channel, Communi::IrcMessage *message, + const std::vector &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 newThread = std::make_shared(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,7 +330,7 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message, auto thread = parentThreadIt->second.lock(); if (thread) { - builder.setParent(thread->root()); + ctx.parent = thread->root(); } } else @@ -332,12 +338,14 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message, auto parent = channel->findMessage(parentID); if (parent) { - builder.setParent(parent); + ctx.parent = parent; } } } } } + + return ctx; } std::optional parseClearChatMessage( @@ -705,7 +713,13 @@ std::vector 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(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,7 +1590,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message, auto thread = parentThreadIt->second.lock(); if (thread) { - builder.setParent(thread->root()); + replyCtx.parent = thread->root(); } } else @@ -1585,13 +1598,23 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message, 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)