Fixed comma appended to username completion when not at the beginning of the message (#3060)

Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Minor: Received IRC messages use `time` message tag for timestamp if it's available. (#3021)
 - Minor: Added informative messages for recent-messages API's errors. (#3029)
 - Bugfix: Fixed "smiley" emotes being unable to be "Tabbed" with autocompletion, introduced in v2.3.3. (#3010)
+- Bugfix: Fixed comma appended to username completion when not at the beginning of the message. (#3060)
 - Dev: Ubuntu packages are now available (#2936)
 
 ## 2.3.3

src/common/CompletionModel.cpp

@@ -10,6 +10,7 @@
 #include "providers/twitch/TwitchIrcServer.hpp"
 #include "singletons/Emotes.hpp"
 #include "singletons/Settings.hpp"
+#include "util/Helpers.hpp"
 #include "util/QStringHash.hpp"
 
 #include <QtAlgorithms>
@@ -150,9 +151,6 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord)
     }
 
     // Usernames
-    QString usernamePostfix =
-        isFirstWord && getSettings()->mentionUsersWithComma ? "," : QString();
-
     if (prefix.startsWith("@"))
     {
         QString usernamePrefix = prefix;
@@ -162,8 +160,10 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord)
 
         for (const auto &name : chatters)
         {
-            addString("@" + name + usernamePostfix,
+            addString(
-                      TaggedString::Type::Username);
+                "@" + formatUserMention(name, isFirstWord,
+                                        getSettings()->mentionUsersWithComma),
+                TaggedString::Type::Username);
         }
     }
     else if (!getSettings()->userCompletionOnlyWithAt)
@@ -172,7 +172,9 @@ void CompletionModel::refresh(const QString &prefix, bool isFirstWord)
 
         for (const auto &name : chatters)
         {
-            addString(name + usernamePostfix, TaggedString::Type::Username);
+            addString(formatUserMention(name, isFirstWord,
+                                        getSettings()->mentionUsersWithComma),
+                      TaggedString::Type::Username);
         }
     }
 

src/util/Helpers.cpp

@@ -68,4 +68,17 @@ QColor getRandomColor(const QString &userId)
     return TWITCH_USERNAME_COLORS[colorIndex];
 }
 
+QString formatUserMention(const QString &userName, bool isFirstWord,
+                          bool mentionUsersWithComma)
+{
+    QString result = userName;
+
+    if (isFirstWord && mentionUsersWithComma)
+    {
+        result += ",";
+    }
+
+    return result;
+}
+
 }  // namespace chatterino

src/util/Helpers.hpp

@@ -20,4 +20,14 @@ QString kFormatNumbers(const int &number);
 
 QColor getRandomColor(const QString &userId);
 
+/**
+ * @brief Takes a user's name and some formatting parameter and spits out the standardized way to format it
+ *
+ * @param userName a user's name
+ * @param isFirstWord signifies whether this mention would be the first word in a message
+ * @param mentionUsersWithComma postfix mentions with a comma. generally powered by getSettings()->mentionUsersWithComma
+ **/
+QString formatUserMention(const QString &userName, bool isFirstWord,
+                          bool mentionUsersWithComma);
+
 }  // namespace chatterino

src/widgets/helper/ChannelView.cpp

@@ -37,6 +37,7 @@
 #include "singletons/WindowManager.hpp"
 #include "util/Clipboard.hpp"
 #include "util/DistanceBetweenPoints.hpp"
+#include "util/Helpers.hpp"
 #include "util/IncognitoBrowser.hpp"
 #include "util/StreamerMode.hpp"
 #include "util/Twitch.hpp"
@@ -48,6 +49,7 @@
 #include "widgets/helper/EffectLabel.hpp"
 #include "widgets/helper/SearchPopup.hpp"
 #include "widgets/splits/Split.hpp"
+#include "widgets/splits/SplitInput.hpp"
 
 #define DRAW_WIDTH (this->width())
 #define SELECTION_RESUME_SCROLLING_MSG_THRESHOLD 3
@@ -1804,8 +1806,9 @@ void ChannelView::handleMouseClick(QMouseEvent *event,
         }
         break;
         case Qt::RightButton: {
+            auto split = dynamic_cast<Split *>(this->parentWidget());
             auto insertText = [=](QString text) {
-                if (auto split = dynamic_cast<Split *>(this->parentWidget()))
+                if (split)
                 {
                     split->insertTextToInput(text);
                 }
@@ -1815,7 +1818,11 @@ void ChannelView::handleMouseClick(QMouseEvent *event,
             if (link.type == Link::UserInfo)
             {
                 const bool commaMention = getSettings()->mentionUsersWithComma;
-                insertText("@" + link.value + (commaMention ? ", " : " "));
+                const bool isFirstWord =
+                    split && split->getInput().isEditFirstWord();
+                auto userMention =
+                    formatUserMention(link.value, isFirstWord, commaMention);
+                insertText("@" + userMention + " ");
             }
             else if (link.type == Link::UserWhisper)
             {

src/widgets/helper/ResizingTextEdit.cpp

@@ -39,6 +39,19 @@ bool ResizingTextEdit::hasHeightForWidth() const
     return true;
 }
 
+bool ResizingTextEdit::isFirstWord() const
+{
+    QString plainText = this->toPlainText();
+    for (int i = this->textCursor().position(); i >= 0; i--)
+    {
+        if (plainText[i] == ' ')
+        {
+            return false;
+        }
+    }
+    return true;
+};
+
 int ResizingTextEdit::heightForWidth(int) const
 {
     auto margins = this->contentsMargins();
@@ -108,17 +121,6 @@ void ResizingTextEdit::keyPressEvent(QKeyEvent *event)
         }
 
         QString currentCompletionPrefix = this->textUnderCursor();
-        bool isFirstWord = [&] {
-            QString plainText = this->toPlainText();
-            for (int i = this->textCursor().position(); i >= 0; i--)
-            {
-                if (plainText[i] == ' ')
-                {
-                    return false;
-                }
-            }
-            return true;
-        }();
 
         // check if there is something to complete
         if (currentCompletionPrefix.size() <= 1)
@@ -134,7 +136,8 @@ void ResizingTextEdit::keyPressEvent(QKeyEvent *event)
             // First type pressing tab after modifying a message, we refresh our
             // completion model
             this->completer_->setModel(completionModel);
-            completionModel->refresh(currentCompletionPrefix, isFirstWord);
+            completionModel->refresh(currentCompletionPrefix,
+                                     this->isFirstWord());
             this->completionInProgress_ = true;
             this->completer_->setCompletionPrefix(currentCompletionPrefix);
             this->completer_->complete();

src/widgets/helper/ResizingTextEdit.hpp

@@ -15,6 +15,7 @@ public:
     QSize sizeHint() const override;
 
     bool hasHeightForWidth() const override;
+    bool isFirstWord() const;
 
     pajlada::Signals::Signal<QKeyEvent *> keyPressed;
     pajlada::Signals::NoArgSignal focused;

src/widgets/splits/Split.cpp

@@ -306,6 +306,11 @@ ChannelView &Split::getChannelView()
     return *this->view_;
 }
 
+SplitInput &Split::getInput()
+{
+    return *this->input_;
+}
+
 void Split::updateInputPlaceholder()
 {
     if (!this->getChannel()->isTwitchChannel())

src/widgets/splits/Split.hpp

@@ -47,6 +47,7 @@ public:
     pajlada::Signals::NoArgSignal focusLost;
 
     ChannelView &getChannelView();
+    SplitInput &getInput();
 
     IndirectChannel getIndirectChannel();
     ChannelPtr getChannel();

src/widgets/splits/SplitInput.cpp

@@ -8,6 +8,7 @@
 #include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
 #include "util/Clamp.hpp"
+#include "util/Helpers.hpp"
 #include "util/LayoutCreator.hpp"
 #include "widgets/Notebook.hpp"
 #include "widgets/Scrollbar.hpp"
@@ -568,8 +569,10 @@ void SplitInput::insertCompletionText(const QString &input_)
         }
         else if (text[i] == '@')
         {
-            input = "@" + input_ +
+            const auto userMention =
-                    (getSettings()->mentionUsersWithComma ? ", " : " ");
+                formatUserMention(input_, edit.isFirstWord(),
+                                  getSettings()->mentionUsersWithComma);
+            input = "@" + userMention + " ";
             done = true;
         }
 
@@ -595,6 +598,11 @@ void SplitInput::clearSelection()
     this->ui_.textEdit->setTextCursor(c);
 }
 
+bool SplitInput::isEditFirstWord() const
+{
+    return this->ui_.textEdit->isFirstWord();
+}
+
 QString SplitInput::getInputText() const
 {
     return this->ui_.textEdit->toPlainText();

src/widgets/splits/SplitInput.hpp

@@ -28,6 +28,7 @@ public:
     SplitInput(Split *_chatWidget);
 
     void clearSelection();
+    bool isEditFirstWord() const;
     QString getInputText() const;
     void insertText(const QString &text);
 

tests/CMakeLists.txt

@@ -10,6 +10,7 @@ set(test_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/Emojis.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/ExponentialBackoff.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/TwitchAccount.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/Helpers.cpp
     )
 
 add_executable(${PROJECT_NAME} ${test_SOURCES})

tests/src/Helpers.cpp

@@ -0,0 +1,22 @@
+#include "util/Helpers.hpp"
+
+#include <gtest/gtest.h>
+
+using namespace chatterino;
+
+TEST(Helpers, formatUserMention)
+{
+    const auto userName = "pajlada";
+
+    // A user mention that is the first word, that has 'mention with comma' enabled should have a comma appended at the end.
+    EXPECT_EQ(formatUserMention(userName, true, true), "pajlada,");
+
+    // A user mention that is not the first word, but has 'mention with comma' enabled should not have a comma appended at the end.
+    EXPECT_EQ(formatUserMention(userName, false, true), "pajlada");
+
+    // A user mention that is the first word, but has 'mention with comma' disabled should not have a comma appended at the end.
+    EXPECT_EQ(formatUserMention(userName, true, false), "pajlada");
+
+    // A user mention that is neither the first word, nor has 'mention with comma' enabled should not have a comma appended at the end.
+    EXPECT_EQ(formatUserMention(userName, false, false), "pajlada");
+}