Improve reply popup after thread update (#4923)

Co-authored-by: nerix <nero.9@hotmail.de>
Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Minor: The account switcher is now styled to match your theme. (#4817)
 - Minor: Add an invisible resize handle to the bottom of frameless user info popups and reply thread popups. (#4795)
 - Minor: The installer now checks for the VC Runtime version and shows more info when it's outdated. (#4847)
+- Minor: Add menu actions to reply directly to a message or the original thread root. (#4923)
 - Minor: The `/reply` command now replies to the latest message of the user. (#4919)
 - Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840)
 - Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848)
@@ -29,6 +30,7 @@
 - Bugfix: Fixed headers of tables in the settings switching to bold text when selected. (#4913)
 - Bugfix: Fixed tooltips appearing too large and/or away from the cursor. (#4920)
 - Bugfix: Fixed a crash when clicking `More messages below` button in a usercard and closing it quickly. (#4933)
+- Bugfix: Fixed thread popup window missing messages for nested threads. (#4923)
 - Dev: Change clang-format from v14 to v16. (#4929)
 - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791)
 - Dev: Temporarily disable High DPI scaling on Qt6 builds on Windows. (#4767)

src/messages/Message.hpp

@@ -53,6 +53,8 @@ enum class MessageFlag : int64_t {
 };
 using MessageFlags = FlagsEnum<MessageFlag>;
 
+struct Message;
+using MessagePtr = std::shared_ptr<const Message>;
 struct Message {
     Message();
     ~Message();
@@ -88,12 +90,11 @@ struct Message {
     // the reply thread will be cleaned up by the TwitchChannel.
     // The root of the thread does not have replyThread set.
     std::shared_ptr<MessageThread> replyThread;
+    MessagePtr replyParent;
     uint32_t count = 1;
     std::vector<std::unique_ptr<MessageElement>> elements;
 
     ScrollbarHighlight getScrollBarHighlight() const;
 };
 
-using MessagePtr = std::shared_ptr<const Message>;
-
 }  // namespace chatterino

src/providers/twitch/IrcMessageHandler.cpp

@@ -243,10 +243,12 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
                    TwitchMessageBuilder &builder)
 {
     const auto &tags = message->tags();
-    if (const auto it = tags.find("reply-parent-msg-id"); it != tags.end())
+    if (const auto it = tags.find("reply-thread-parent-msg-id");
+        it != tags.end())
     {
         const QString replyID = it.value().toString();
         auto threadIt = channel->threads().find(replyID);
+        std::shared_ptr<MessageThread> rootThread;
         if (threadIt != channel->threads().end())
         {
             auto owned = threadIt->second.lock();
@@ -256,43 +258,80 @@ void populateReply(TwitchChannel *channel, Communi::IrcMessage *message,
                 updateReplyParticipatedStatus(tags, message->nick(), builder,
                                               owned, false);
                 builder.setThread(owned);
-                return;
+                rootThread = owned;
             }
         }
 
-        MessagePtr foundMessage;
-
-        // Thread does not yet exist, find root reply and create thread.
-        // Linear search is justified by the infrequent use of replies
-        for (const auto &otherMsg : otherLoaded)
+        if (!rootThread)
         {
-            if (otherMsg->id == replyID)
+            MessagePtr foundMessage;
+
+            // Thread does not yet exist, find root reply and create thread.
+            // Linear search is justified by the infrequent use of replies
+            for (const auto &otherMsg : otherLoaded)
             {
-                // Found root reply message
-                foundMessage = otherMsg;
-                break;
+                if (otherMsg->id == replyID)
+                {
+                    // Found root reply message
+                    foundMessage = otherMsg;
+                    break;
+                }
+            }
+
+            if (!foundMessage)
+            {
+                // We didn't find the reply root message in the otherLoaded messages
+                // which are typically the already-parsed recent messages from the
+                // Recent Messages API. We could have a really old message that
+                // still exists being replied to, so check for that here.
+                foundMessage = channel->findMessage(replyID);
+            }
+
+            if (foundMessage)
+            {
+                std::shared_ptr<MessageThread> newThread =
+                    std::make_shared<MessageThread>(foundMessage);
+                updateReplyParticipatedStatus(tags, message->nick(), builder,
+                                              newThread, true);
+
+                builder.setThread(newThread);
+                rootThread = newThread;
+                // Store weak reference to thread in channel
+                channel->addReplyThread(newThread);
             }
         }
 
-        if (!foundMessage)
+        if (const auto parentIt = tags.find("reply-parent-msg-id");
+            parentIt != tags.end())
         {
-            // We didn't find the reply root message in the otherLoaded messages
-            // which are typically the already-parsed recent messages from the
-            // Recent Messages API. We could have a really old message that
-            // still exists being replied to, so check for that here.
-            foundMessage = channel->findMessage(replyID);
-        }
-
-        if (foundMessage)
-        {
-            std::shared_ptr<MessageThread> newThread =
-                std::make_shared<MessageThread>(foundMessage);
-            updateReplyParticipatedStatus(tags, message->nick(), builder,
-                                          newThread, true);
-
-            builder.setThread(newThread);
-            // Store weak reference to thread in channel
-            channel->addReplyThread(newThread);
+            const QString parentID = parentIt.value().toString();
+            if (replyID == parentID)
+            {
+                if (rootThread)
+                {
+                    builder.setParent(rootThread->root());
+                }
+            }
+            else
+            {
+                auto parentThreadIt = channel->threads().find(parentID);
+                if (parentThreadIt != channel->threads().end())
+                {
+                    auto thread = parentThreadIt->second.lock();
+                    if (thread)
+                    {
+                        builder.setParent(thread->root());
+                    }
+                }
+                else
+                {
+                    auto parent = channel->findMessage(parentID);
+                    if (parent)
+                    {
+                        builder.setParent(parent);
+                    }
+                }
+            }
         }
     }
 }
@@ -1283,17 +1322,20 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
     TwitchMessageBuilder builder(chan.get(), message, args, content, isAction);
     builder.setMessageOffset(messageOffset);
 
-    if (const auto it = tags.find("reply-parent-msg-id"); it != tags.end())
+    if (const auto it = tags.find("reply-thread-parent-msg-id");
+        it != tags.end())
     {
         const QString replyID = it.value().toString();
-        auto threadIt = channel->threads_.find(replyID);
-        if (threadIt != channel->threads_.end() && !threadIt->second.expired())
+        auto threadIt = channel->threads().find(replyID);
+        std::shared_ptr<MessageThread> rootThread;
+        if (threadIt != channel->threads().end() && !threadIt->second.expired())
         {
             // Thread already exists (has a reply)
             auto thread = threadIt->second.lock();
             updateReplyParticipatedStatus(tags, message->nick(), builder,
                                           thread, false);
             builder.setThread(thread);
+            rootThread = thread;
         }
         else
         {
@@ -1307,10 +1349,44 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
                                               newThread, true);
 
                 builder.setThread(newThread);
+                rootThread = newThread;
                 // Store weak reference to thread in channel
                 channel->addReplyThread(newThread);
             }
         }
+
+        if (const auto parentIt = tags.find("reply-parent-msg-id");
+            parentIt != tags.end())
+        {
+            const QString parentID = parentIt.value().toString();
+            if (replyID == parentID)
+            {
+                if (rootThread)
+                {
+                    builder.setParent(rootThread->root());
+                }
+            }
+            else
+            {
+                auto parentThreadIt = channel->threads().find(parentID);
+                if (parentThreadIt != channel->threads().end())
+                {
+                    auto thread = parentThreadIt->second.lock();
+                    if (thread)
+                    {
+                        builder.setParent(thread->root());
+                    }
+                }
+                else
+                {
+                    auto parent = channel->findMessage(parentID);
+                    if (parent)
+                    {
+                        builder.setParent(parent);
+                    }
+                }
+            }
+        }
     }
 
     if (isSub || !builder.isIgnored())

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -623,15 +623,24 @@ void TwitchMessageBuilder::parseThread()
     {
         // set references
         this->message().replyThread = this->thread_;
+        this->message().replyParent = this->parent_;
         this->thread_->addToThread(this->weakOf());
 
         // enable reply flag
         this->message().flags.set(MessageFlag::ReplyMessage);
 
-        const auto &threadRoot = this->thread_->root();
+        MessagePtr threadRoot;
+        if (!this->parent_)
+        {
+            threadRoot = this->thread_->root();
+        }
+        else
+        {
+            threadRoot = this->parent_;
+        }
 
         QString usernameText = SharedMessageBuilder::stylizeUsername(
-            threadRoot->loginName, *threadRoot.get());
+            threadRoot->loginName, *threadRoot);
 
         this->emplace<ReplyCurveElement>();
 
@@ -1811,6 +1820,11 @@ void TwitchMessageBuilder::setThread(std::shared_ptr<MessageThread> thread)
     this->thread_ = std::move(thread);
 }
 
+void TwitchMessageBuilder::setParent(MessagePtr parent)
+{
+    this->parent_ = std::move(parent);
+}
+
 void TwitchMessageBuilder::setMessageOffset(int offset)
 {
     this->messageOffset_ = offset;

src/providers/twitch/TwitchMessageBuilder.hpp

@@ -58,6 +58,7 @@ public:
     MessagePtr build() override;
 
     void setThread(std::shared_ptr<MessageThread> thread);
+    void setParent(MessagePtr parent);
     void setMessageOffset(int offset);
 
     static void appendChannelPointRewardMessage(
@@ -131,6 +132,7 @@ private:
     bool bitsStacked = false;
     bool historicalMessage_ = false;
     std::shared_ptr<MessageThread> thread_;
+    MessagePtr parent_;
 
     /**
      * Starting offset to be used on index-based operations on `originalMessage_`.

src/singletons/helper/LoggingChannel.cpp

@@ -132,8 +132,17 @@ void LoggingChannel::addMessage(MessagePtr message)
         qsizetype colonIndex = messageText.indexOf(':');
         if (colonIndex != -1)
         {
-            QString rootMessageChatter =
-                message->replyThread->root()->loginName;
+            QString rootMessageChatter;
+            if (message->replyParent)
+            {
+                rootMessageChatter = message->replyParent->loginName;
+            }
+            else
+            {
+                // we actually want to use 'reply-parent-user-login' tag here,
+                // but it's not worth storing just for this edge case
+                rootMessageChatter = message->replyThread->root()->loginName;
+            }
             messageText.insert(colonIndex + 1, " @" + rootMessageChatter);
         }
     }

src/widgets/dialogs/ReplyThreadPopup.cpp

@@ -189,7 +189,7 @@ ReplyThreadPopup::ReplyThreadPopup(bool closeAutomatically, QWidget *parent,
 void ReplyThreadPopup::setThread(std::shared_ptr<MessageThread> thread)
 {
     this->thread_ = std::move(thread);
-    this->ui_.replyInput->setReply(this->thread_);
+    this->ui_.replyInput->setReply(this->thread_->root());
     this->addMessagesFromThread();
     this->updateInputUI();
 

src/widgets/helper/ChannelView.cpp

@@ -2331,6 +2331,10 @@ void ChannelView::addMessageContextMenuItems(QMenu *menu,
 
         if (messagePtr->replyThread != nullptr)
         {
+            menu->addAction("Reply to &original thread", [this, &messagePtr] {
+                this->setInputReply(messagePtr->replyThread->root());
+            });
+
             menu->addAction("View &thread", [this, &messagePtr] {
                 this->showReplyThreadPopup(messagePtr);
             });
@@ -2871,19 +2875,17 @@ void ChannelView::setInputReply(const MessagePtr &message)
         return;
     }
 
-    auto thread = message->replyThread;
-
-    if (!thread)
+    if (!message->replyThread)
     {
         // Message did not already have a thread attached, try to find or create one
         if (auto *tc =
                 dynamic_cast<TwitchChannel *>(this->underlyingChannel_.get()))
         {
-            thread = tc->getOrCreateThread(message);
+            tc->getOrCreateThread(message);
         }
         else if (auto *tc = dynamic_cast<TwitchChannel *>(this->channel_.get()))
         {
-            thread = tc->getOrCreateThread(message);
+            tc->getOrCreateThread(message);
         }
         else
         {
@@ -2894,7 +2896,7 @@ void ChannelView::setInputReply(const MessagePtr &message)
         }
     }
 
-    this->split_->setInputReply(thread);
+    this->split_->setInputReply(message);
 }
 
 void ChannelView::showReplyThreadPopup(const MessagePtr &message)

src/widgets/splits/Split.cpp

@@ -1542,7 +1542,7 @@ void Split::drag()
     stopDraggingSplit();
 }
 
-void Split::setInputReply(const std::shared_ptr<MessageThread> &reply)
+void Split::setInputReply(const MessagePtr &reply)
 {
     this->input_->setReply(reply);
 }

src/widgets/splits/Split.hpp

@@ -15,7 +15,6 @@
 namespace chatterino {
 
 class ChannelView;
-class MessageThread;
 class SplitHeader;
 class SplitInput;
 class SplitContainer;
@@ -75,7 +74,7 @@ public:
 
     void setContainer(SplitContainer *container);
 
-    void setInputReply(const std::shared_ptr<MessageThread> &reply);
+    void setInputReply(const MessagePtr &reply);
 
     static pajlada::Signals::Signal<Qt::KeyboardModifiers>
         modifierStatusChanged;

src/widgets/splits/SplitInput.cpp

@@ -359,7 +359,7 @@ QString SplitInput::handleSendMessage(std::vector<QString> &arguments)
         if (this->enableInlineReplying_)
         {
             // Remove @username prefix that is inserted when doing inline replies
-            message.remove(0, this->replyThread_->root()->displayName.length() +
+            message.remove(0, this->replyThread_->displayName.length() +
                                   1);  // remove "@username"
 
             if (!message.isEmpty() && message.at(0) == ' ')
@@ -373,7 +373,7 @@ QString SplitInput::handleSendMessage(std::vector<QString> &arguments)
             getApp()->commands->execCommand(message, c, false);
 
         // Reply within TwitchChannel
-        tc->sendReply(sendMessage, this->replyThread_->rootId());
+        tc->sendReply(sendMessage, this->replyThread_->id);
 
         this->postMessageSend(message, arguments);
         return "";
@@ -992,7 +992,7 @@ void SplitInput::editTextChanged()
             // We need to verify that
             // 1. the @username prefix exists and
             // 2. if a character exists after the @username, it is a space
-            QString replyPrefix = "@" + this->replyThread_->root()->displayName;
+            QString replyPrefix = "@" + this->replyThread_->displayName;
             if (!text.startsWith(replyPrefix) ||
                 (text.length() > replyPrefix.length() &&
                  text.at(replyPrefix.length()) != ' '))
@@ -1065,15 +1065,29 @@ void SplitInput::giveFocus(Qt::FocusReason reason)
     this->ui_.textEdit->setFocus(reason);
 }
 
-void SplitInput::setReply(std::shared_ptr<MessageThread> reply,
-                          bool showReplyingLabel)
+void SplitInput::setReply(MessagePtr reply, bool showReplyingLabel)
 {
+    auto oldParent = this->replyThread_;
+    if (this->enableInlineReplying_ && oldParent)
+    {
+        // Remove old reply prefix
+        auto replyPrefix = "@" + oldParent->displayName;
+        auto plainText = this->ui_.textEdit->toPlainText().trimmed();
+        if (plainText.startsWith(replyPrefix))
+        {
+            plainText.remove(0, replyPrefix.length());
+        }
+        this->ui_.textEdit->setPlainText(plainText.trimmed());
+        this->ui_.textEdit->moveCursor(QTextCursor::EndOfBlock);
+        this->ui_.textEdit->resetCompletion();
+    }
+
     this->replyThread_ = std::move(reply);
 
     if (this->enableInlineReplying_)
     {
         // Only enable reply label if inline replying
-        auto replyPrefix = "@" + this->replyThread_->root()->displayName;
+        auto replyPrefix = "@" + this->replyThread_->displayName;
         auto plainText = this->ui_.textEdit->toPlainText().trimmed();
         if (!plainText.startsWith(replyPrefix))
         {
@@ -1086,7 +1100,7 @@ void SplitInput::setReply(std::shared_ptr<MessageThread> reply,
             this->ui_.textEdit->resetCompletion();
         }
         this->ui_.replyLabel->setText("Replying to @" +
-                                      this->replyThread_->root()->displayName);
+                                      this->replyThread_->displayName);
     }
 }
 

src/widgets/splits/SplitInput.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "messages/Message.hpp"
 #include "widgets/BaseWidget.hpp"
 
 #include <QHBoxLayout>
@@ -19,7 +20,6 @@ class Split;
 class EmotePopup;
 class InputCompletionPopup;
 class EffectLabel;
-class MessageThread;
 class ResizingTextEdit;
 class ChannelView;
 enum class CompletionKind;
@@ -40,8 +40,7 @@ public:
     QString getInputText() const;
     void insertText(const QString &text);
 
-    void setReply(std::shared_ptr<MessageThread> reply,
-                  bool showInlineReplying = true);
+    void setReply(MessagePtr reply, bool showInlineReplying = true);
     void setPlaceholderText(const QString &text);
 
     /**
@@ -135,7 +134,7 @@ protected:
         EffectLabel *cancelReplyButton;
     } ui_{};
 
-    std::shared_ptr<MessageThread> replyThread_ = nullptr;
+    MessagePtr replyThread_ = nullptr;
     bool enableInlineReplying_;
 
     pajlada::Signals::SignalHolder managedConnections_;