fix: support completing emotes starting with : (Chatterino#5603)

imanol-s · Oct 6, 2024 · 9ba7ef3 · 9ba7ef3
1 parent 0085fb6
commit 9ba7ef3
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,7 @@
 - Bugfix: Fixed grammar in the user highlight page. (#5602)
 - Bugfix: Fixed incorrect message being disabled in some cases upon approving or denying an automod caught message. (#5611)
 - Bugfix: Fixed double-click selection not working when clicking outside a message. (#5617)
+- Bugfix: Fixed emotes starting with ":" not tab-completing. (#5603)
 - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420)
 - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422)
 - Dev: Unsingletonize `ISoundController`. (#5462)

diff --git a/src/controllers/completion/strategies/ClassicEmoteStrategy.cpp b/src/controllers/completion/strategies/ClassicEmoteStrategy.cpp
@@ -52,25 +52,36 @@ void ClassicTabEmoteStrategy::apply(const std::vector<EmoteItem> &items,
                                     std::vector<EmoteItem> &output,
                                     const QString &query) const
 {
-    bool emojiOnly = false;
-    QString normalizedQuery = query;
-    if (normalizedQuery.startsWith(':'))
+    bool colonStart = query.startsWith(':');
+    QStringView normalizedQuery = query;
+    if (colonStart)
     {
+        // TODO(Qt6): use sliced
         normalizedQuery = normalizedQuery.mid(1);
-        // tab completion with : prefix should do emojis only
-        emojiOnly = true;
     }
 
     std::set<EmoteItem, CompletionEmoteOrder> emotes;
 
     for (const auto &item : items)
     {
-        if (emojiOnly ^ item.isEmoji)
+        QStringView itemQuery;
+        if (item.isEmoji)
+        {
+            if (colonStart)
+            {
+                itemQuery = normalizedQuery;
+            }
+            else
+            {
+                continue;  // ignore emojis when not completing with ':'
+            }
+        }
+        else
         {
-            continue;
+            itemQuery = query;
         }
 
-        if (startsWithOrContains(item.searchName, normalizedQuery,
+        if (startsWithOrContains(item.searchName, itemQuery,
                                  Qt::CaseInsensitive,
                                  getSettings()->prefixOnlyEmoteCompletion))
         {

diff --git a/src/controllers/completion/strategies/SmartEmoteStrategy.cpp b/src/controllers/completion/strategies/SmartEmoteStrategy.cpp
@@ -25,8 +25,7 @@ namespace {
      * @return How different the emote is from query. Values in the range [-10,
      * \infty].
      */
-    int costOfEmote(const QString &query, const QString &emote,
-                    bool prioritizeUpper)
+    int costOfEmote(QStringView query, QStringView emote, bool prioritizeUpper)
     {
         int score = 0;
 
@@ -68,8 +67,8 @@ namespace {
     // matchingFunction is used for testing if the emote should be included in the search.
     void completeEmotes(
         const std::vector<EmoteItem> &items, std::vector<EmoteItem> &output,
-        const QString &query, bool ignoreColonForCost,
-        const std::function<bool(EmoteItem, QString, Qt::CaseSensitivity)>
+        QStringView query, bool ignoreColonForCost,
+        const std::function<bool(EmoteItem, Qt::CaseSensitivity)>
             &matchingFunction)
     {
         // Given these emotes: pajaW, PAJAW
@@ -92,8 +91,7 @@ namespace {
         for (const auto &item : items)
         {
             if (matchingFunction(
-                    item, query,
-                    haveUpper ? Qt::CaseSensitive : Qt::CaseInsensitive))
+                    item, haveUpper ? Qt::CaseSensitive : Qt::CaseInsensitive))
             {
                 output.push_back(item);
             }
@@ -118,7 +116,7 @@ namespace {
             // Run the search again but this time without case sensitivity
             for (const auto &item : items)
             {
-                if (matchingFunction(item, query, Qt::CaseInsensitive))
+                if (matchingFunction(item, Qt::CaseInsensitive))
                 {
                     output.push_back(item);
                 }
@@ -170,35 +168,49 @@ void SmartEmoteStrategy::apply(const std::vector<EmoteItem> &items,
         ignoreColonForCost = true;
     }
     completeEmotes(items, output, normalizedQuery, ignoreColonForCost,
-                   [](const EmoteItem &left, const QString &right,
-                      Qt::CaseSensitivity caseHandling) {
-                       return left.searchName.contains(right, caseHandling);
+                   [normalizedQuery](const EmoteItem &left,
+                                     Qt::CaseSensitivity caseHandling) {
+                       return left.searchName.contains(normalizedQuery,
+                                                       caseHandling);
                    });
 }
 
 void SmartTabEmoteStrategy::apply(const std::vector<EmoteItem> &items,
                                   std::vector<EmoteItem> &output,
                                   const QString &query) const
 {
-    bool emojiOnly = false;
-    QString normalizedQuery = query;
-    if (normalizedQuery.startsWith(':'))
+    bool colonStart = query.startsWith(':');
+    QStringView normalizedQuery = query;
+    if (colonStart)
     {
+        // TODO(Qt6): use sliced
         normalizedQuery = normalizedQuery.mid(1);
-        // tab completion with : prefix should do emojis only
-        emojiOnly = true;
     }
-    completeEmotes(items, output, normalizedQuery, false,
-                   [emojiOnly](const EmoteItem &left, const QString &right,
-                               Qt::CaseSensitivity caseHandling) -> bool {
-                       if (emojiOnly ^ left.isEmoji)
-                       {
-                           return false;
-                       }
-                       return startsWithOrContains(
-                           left.searchName, right, caseHandling,
-                           getSettings()->prefixOnlyEmoteCompletion);
-                   });
+
+    completeEmotes(
+        items, output, normalizedQuery, false,
+        [&](const EmoteItem &item, Qt::CaseSensitivity caseHandling) -> bool {
+            QStringView itemQuery;
+            if (item.isEmoji)
+            {
+                if (colonStart)
+                {
+                    itemQuery = normalizedQuery;
+                }
+                else
+                {
+                    return false;  // ignore emojis when not completing with ':'
+                }
+            }
+            else
+            {
+                itemQuery = query;
+            }
+
+            return startsWithOrContains(
+                item.searchName, itemQuery, caseHandling,
+                getSettings()->prefixOnlyEmoteCompletion);
+        });
 }
 
 }  // namespace chatterino::completion
diff --git a/src/providers/twitch/TwitchAccount.cpp b/src/providers/twitch/TwitchAccount.cpp
@@ -371,7 +371,7 @@ void TwitchAccount::reloadEmotes(void *caller)
         auto meta = getTwitchEmoteSetMeta(emote);
 
         auto emotePtr = twitchEmotes->getOrCreateEmote(id, name);
-        if (!emoteMap->try_emplace(name, emotePtr).second)
+        if (!emoteMap->try_emplace(emotePtr->name, emotePtr).second)
         {
             // if the emote already exists, we don't want to add it to a set as
             // those are assumed to be disjoint

diff --git a/src/util/Helpers.cpp b/src/util/Helpers.cpp
@@ -125,7 +125,7 @@ namespace helpers::detail {
 }  // namespace helpers::detail
 using namespace helpers::detail;
 
-bool startsWithOrContains(const QString &str1, const QString &str2,
+bool startsWithOrContains(QStringView str1, QStringView str2,
                           Qt::CaseSensitivity caseSensitivity, bool startsWith)
 {
     if (startsWith)

diff --git a/src/util/Helpers.hpp b/src/util/Helpers.hpp
@@ -59,7 +59,7 @@ namespace helpers::detail {
  * @brief startsWithOrContains is a wrapper for checking
  * whether str1 starts with or contains str2 within itself
  **/
-bool startsWithOrContains(const QString &str1, const QString &str2,
+bool startsWithOrContains(QStringView str1, QStringView str2,
                           Qt::CaseSensitivity caseSensitivity, bool startsWith);
 
 /**

diff --git a/tests/src/InputCompletion.cpp b/tests/src/InputCompletion.cpp
@@ -387,10 +387,12 @@ TEST_F(InputCompletionTest, ClassicTabCompletionEmote)
 TEST_F(InputCompletionTest, ClassicTabCompletionEmoji)
 {
     auto completion = queryClassicTabCompletion(":tf", false);
-    ASSERT_EQ(completion.size(), 0);
+    ASSERT_EQ(completion.size(), 1);
+    ASSERT_EQ(completion[0], ":tf: ");
 
     completion = queryClassicTabCompletion(":)", false);
-    ASSERT_EQ(completion.size(), 0);
+    ASSERT_EQ(completion.size(), 1);
+    ASSERT_EQ(completion[0], ":) ");
 
     completion = queryClassicTabCompletion(":cla", false);
     ASSERT_EQ(completion.size(), 8);
@@ -536,12 +538,12 @@ TEST_F(InputCompletionTest, SmartTabCompletionEmote)
 TEST_F(InputCompletionTest, SmartTabCompletionEmoji)
 {
     auto completion = querySmartTabCompletion(":tf", false);
-    ASSERT_EQ(completion.size(), 0);
-    // ASSERT_EQ(completion[0], ":tf: ");
+    ASSERT_EQ(completion.size(), 1);
+    ASSERT_EQ(completion[0], ":tf: ");
 
     completion = querySmartTabCompletion(":)", false);
-    ASSERT_EQ(completion.size(), 0);
-    // ASSERT_EQ(completion[0], ":) ");
+    ASSERT_EQ(completion.size(), 1);
+    ASSERT_EQ(completion[0], ":) ");
 
     completion = querySmartTabCompletion(":cla", false);
     ASSERT_EQ(completion.size(), 8);