Improvements to Message Search (#1237)

* Ran clang-format

* Implement user-specific search in message history

This functionality was originally requested in #1236.

This commit changes the SearchPopup::performSearch method so that only
messages from specific users can be shown.

In order to filter for a specific user, enter their username with a
leading '@' in the search popup. You can also add an additional search
phrase which will also be considered in the search.

* Naive implementation for "from:" tags

Rebase later?

* Cleverer (?) version using Predicates

Commit adds two POC predicates: one for the author of messages, and one
for substring search in messages.

Problems/TODOs:
* Best way to register new predicates?
* Clean up tags (e.g. "from:") or not?
* Test combinations of different predicates

* Add a predicate to check for links in messages

* Remove a dumb TODO

* Rewrite SearchPopup::performSearch to be cleaner

* Ran clang-format on all files

* Remove TODO I missed earlier

* Forgot to run clang-format

peepoSadDank

* Re-use {}-initialization

Was accidentally removed when fixing earlier merge conflict.

* Does this fix line endings?

No diffs are shown locally, hopefully Git doesn't lie to me.

* Rename "predicates" directory to "search"

Resolving one conversation in the review of #1237.

* Use LinkParser in LinkPredicate

Resolving a conversation in the review of #1237.

* Predicates: Use unique_ptr instead of shared_ptr

Resolves a conversation in the review of #1237.

* Refactor of SearchPopup and AuthorPredicate

Resolving some points from the review in #1237.

* Moved parsing of comma-seperated values into AuthorPredicate
  constructor.
* Rewrite SearchPopup::parsePredicates as suggested.
* Deleted now redundant methods in SearchPopup.

* MessagePredicate::appliesTo now takes a Message&

... instead of a MessagePtr.

This resolves a conversation in the review of #1237.

* Run clang-format on two files I missed

* AuthorPredicate: Check for displayName & loginName

Resolving conversation on #1237.

chatterino.pro

@@ -122,6 +122,9 @@ SOURCES += \
     src/messages/MessageColor.cpp \
     src/messages/MessageContainer.cpp \
     src/messages/MessageElement.cpp \
+    src/messages/search/AuthorPredicate.cpp \
+    src/messages/search/LinkPredicate.cpp \
+    src/messages/search/SubstringPredicate.cpp \
     src/providers/bttv/BttvEmotes.cpp \
     src/providers/bttv/LoadBttvChannelEmote.cpp \
     src/providers/chatterino/ChatterinoBadges.cpp \
@@ -285,6 +288,10 @@ HEADERS += \
     src/messages/MessageContainer.hpp \
     src/messages/MessageElement.hpp \
     src/messages/MessageParseArgs.hpp \
+    src/messages/search/AuthorPredicate.hpp \
+    src/messages/search/LinkPredicate.hpp \
+    src/messages/search/MessagePredicate.hpp \
+    src/messages/search/SubstringPredicate.hpp \
     src/messages/Selection.hpp \
     src/PrecompiledHeader.hpp \
     src/providers/bttv/BttvEmotes.hpp \

src/messages/search/AuthorPredicate.cpp

@@ -0,0 +1,24 @@
+#include "messages/search/AuthorPredicate.hpp"
+
+namespace chatterino {
+
+AuthorPredicate::AuthorPredicate(const QStringList &authors)
+    : authors_()
+{
+    // Check if any comma-seperated values were passed and transform those
+    for (const auto &entry : authors)
+    {
+        for (const auto &author : entry.split(',', QString::SkipEmptyParts))
+        {
+            this->authors_ << author;
+        }
+    }
+}
+
+bool AuthorPredicate::appliesTo(const Message &message)
+{
+    return authors_.contains(message.displayName, Qt::CaseInsensitive) ||
+           authors_.contains(message.loginName, Qt::CaseInsensitive);
+}
+
+}  // namespace chatterino

src/messages/search/AuthorPredicate.hpp

@@ -0,0 +1,38 @@
+#pragma once
+
+#include "messages/search/MessagePredicate.hpp"
+
+namespace chatterino {
+
+/**
+ * @brief MessagePredicate checking for the author/sender of a message.
+ *
+ * This predicate will only allow messages that are sent by a list of users,
+ * specified by their user names.
+ */
+class AuthorPredicate : public MessagePredicate
+{
+public:
+    /**
+     * @brief Create an AuthorPredicate with a list of users to search for.
+     *
+     * @param authors a list of user names that a message should be sent from
+     */
+    AuthorPredicate(const QStringList &authors);
+
+    /**
+     * @brief Checks whether the message is authored by any of the users passed
+     *        in the constructor.
+     *
+     * @param message the message to check
+     * @return true if the message was authored by one of the specified users,
+     *         false otherwise
+     */
+    bool appliesTo(const Message &message);
+
+private:
+    /// Holds the user names that will be searched for
+    QStringList authors_;
+};
+
+}  // namespace chatterino

src/messages/search/LinkPredicate.cpp

@@ -0,0 +1,22 @@
+#include "messages/search/LinkPredicate.hpp"
+#include "common/LinkParser.hpp"
+
+namespace chatterino {
+
+LinkPredicate::LinkPredicate()
+{
+}
+
+bool LinkPredicate::appliesTo(const Message &message)
+{
+    for (const auto &word :
+         message.messageText.split(' ', QString::SkipEmptyParts))
+    {
+        if (LinkParser(word).hasMatch())
+            return true;
+    }
+
+    return false;
+}
+
+}  // namespace chatterino

src/messages/search/LinkPredicate.hpp

@@ -0,0 +1,26 @@
+#pragma once
+
+#include "messages/search/MessagePredicate.hpp"
+
+namespace chatterino {
+
+/**
+ * @brief MessagePredicate checking whether a link exists in the message.
+ *
+ * This predicate will only allow messages that contain a link.
+ */
+class LinkPredicate : public MessagePredicate
+{
+public:
+    LinkPredicate();
+
+    /**
+     * @brief Checks whether the message contains a link.
+     *
+     * @param message the message to check
+     * @return true if the message contains a link, false otherwise
+     */
+    bool appliesTo(const Message &message);
+};
+
+}  // namespace chatterino

src/messages/search/MessagePredicate.hpp

@@ -0,0 +1,31 @@
+#pragma once
+
+#include "messages/Message.hpp"
+
+#include <memory>
+
+namespace chatterino {
+
+/**
+ * @brief Abstract base class for message predicates.
+ *
+ * Message predicates define certain features a message can satisfy.
+ * Features are represented by classes derived from this abstract class.
+ * A derived class must override `appliesTo` in order to test for the desired
+ * feature.
+ */
+class MessagePredicate
+{
+public:
+    /**
+     * @brief Checks whether this predicate applies to the passed message.
+     *
+     * Implementations of `appliesTo` should never change the message's content
+     * in order to be compatible with other MessagePredicates.
+     *
+     * @param message the message to check for this predicate
+     * @return true if this predicate applies, false otherwise
+     */
+    virtual bool appliesTo(const Message &message) = 0;
+};
+}  // namespace chatterino

src/messages/search/SubstringPredicate.cpp

@@ -0,0 +1,15 @@
+#include "messages/search/SubstringPredicate.hpp"
+
+namespace chatterino {
+
+SubstringPredicate::SubstringPredicate(const QString &search)
+    : search_(search)
+{
+}
+
+bool SubstringPredicate::appliesTo(const Message &message)
+{
+    return message.messageText.contains(this->search_, Qt::CaseInsensitive);
+}
+
+}  // namespace chatterino

src/messages/search/SubstringPredicate.hpp

@@ -0,0 +1,41 @@
+#pragma once
+
+#include "messages/search/MessagePredicate.hpp"
+
+namespace chatterino {
+
+/**
+ * @brief MessagePredicate checking whether a substring exists in the message.
+ *
+ * This predicate will only allow messages that contain a certain substring in
+ * their `searchText`.
+ */
+class SubstringPredicate : public MessagePredicate
+{
+public:
+    /**
+     * @brief Create a SubstringPredicate with a substring to search for.
+     *
+     * The passed string is searched for case-insensitively.
+     *
+     * @param search the string to search for in the message
+     */
+    SubstringPredicate(const QString &search);
+
+    /**
+     * @brief Checks whether the message contains the substring passed in the
+     *        constructor.
+     *
+     * The check is done case-insensitively.
+     *
+     * @param message the message to check
+     * @return true if the message contains the substring, false otherwise
+     */
+    bool appliesTo(const Message &message);
+
+private:
+    /// Holds the substring to search for in a message's `messageText`
+    const QString search_;
+};
+
+}  // namespace chatterino

src/widgets/dialogs/LogsPopup.cpp

@@ -175,7 +175,8 @@ void LogsPopup::getOverrustleLogs()
                                                  MessageColor::System);
                     builder.emplace<TextElement>(text, MessageElementFlag::Text,
                                                  MessageColor::Text);
-                    builder.message().searchText = text;
+                    builder.message().messageText = text;
+                    builder.message().displayName = this->userName_;
                     messages.push_back(builder.release());
                 }
             }

src/widgets/helper/SearchPopup.cpp

@@ -7,29 +7,45 @@
 
 #include "common/Channel.hpp"
 #include "messages/Message.hpp"
+#include "messages/search/AuthorPredicate.hpp"
+#include "messages/search/LinkPredicate.hpp"
+#include "messages/search/SubstringPredicate.hpp"
 #include "widgets/helper/ChannelView.hpp"
 
 namespace chatterino {
-namespace {
-    ChannelPtr filter(const QString &text, const QString &channelName,
-                      const LimitedQueueSnapshot<MessagePtr> &snapshot)
+
+ChannelPtr SearchPopup::filter(const QString &text, const QString &channelName,
+                               const LimitedQueueSnapshot<MessagePtr> &snapshot)
+{
+    ChannelPtr channel(new Channel(channelName, Channel::Type::None));
+
+    // Parse predicates from tags in "text"
+    auto predicates = parsePredicates(text);
+
+    // Check for every message whether it fulfills all predicates that have
+    // been registered
+    for (size_t i = 0; i < snapshot.size(); ++i)
     {
-        ChannelPtr channel(new Channel(channelName, Channel::Type::None));
+        MessagePtr message = snapshot[i];
 
-        for (size_t i = 0; i < snapshot.size(); i++)
+        bool accept = true;
+        for (const auto &pred : predicates)
         {
-            MessagePtr message = snapshot[i];
-
-            if (text.isEmpty() ||
-                message->searchText.indexOf(text, 0, Qt::CaseInsensitive) != -1)
+            // Discard the message as soon as one predicate fails
+            if (!pred->appliesTo(*message))
             {
-                channel->addMessage(message);
+                accept = false;
+                break;
             }
         }
 
-        return channel;
+        // If all predicates match, add the message to the channel
+        if (accept)
+            channel->addMessage(message);
     }
-}  // namespace
+
+    return channel;
+}
 
 SearchPopup::SearchPopup()
 {
@@ -113,4 +129,55 @@ void SearchPopup::initLayout()
     }
 }
 
+std::vector<std::unique_ptr<MessagePredicate>> SearchPopup::parsePredicates(
+    const QString &input)
+{
+    static QRegularExpression predicateRegex(R"(^(\w+):([\w,]+)$)");
+
+    auto predicates = std::vector<std::unique_ptr<MessagePredicate>>();
+    auto words = input.split(' ', QString::SkipEmptyParts);
+    auto authors = QStringList();
+
+    for (auto it = words.begin(); it != words.end();)
+    {
+        if (auto match = predicateRegex.match(*it); match.hasMatch())
+        {
+            QString name = match.captured(1);
+            QString value = match.captured(2);
+
+            bool remove = true;
+
+            // match predicates
+            if (name == "from")
+            {
+                authors.append(value);
+            }
+            else if (name == "has" && value == "link")
+            {
+                predicates.push_back(std::make_unique<LinkPredicate>());
+            }
+            else
+            {
+                remove = false;
+            }
+
+            // remove or advance
+            it = remove ? words.erase(it) : ++it;
+        }
+        else
+        {
+            ++it;
+        }
+    }
+
+    if (!authors.empty())
+        predicates.push_back(std::make_unique<AuthorPredicate>(authors));
+
+    if (!words.empty())
+        predicates.push_back(
+            std::make_unique<SubstringPredicate>(words.join(" ")));
+
+    return predicates;
+}
+
 }  // namespace chatterino

src/widgets/helper/SearchPopup.hpp

@@ -2,6 +2,7 @@
 
 #include "ForwardDecl.hpp"
 #include "messages/LimitedQueueSnapshot.hpp"
+#include "messages/search/MessagePredicate.hpp"
 #include "widgets/BaseWindow.hpp"
 
 #include <memory>
@@ -26,6 +27,30 @@ private:
     void initLayout();
     void search();
 
+    /**
+     * @brief Only retains those message from a list of messages that satisfy a
+     *        search query.
+     *
+     * @param text          the search query -- will be parsed for MessagePredicates
+     * @param channelName   name of the channel to be returned
+     * @param snapshot      list of messages to filter
+     *
+     * @return a ChannelPtr with "channelName" and the filtered messages from
+     *         "snapshot"
+     */
+    static ChannelPtr filter(const QString &text, const QString &channelName,
+                             const LimitedQueueSnapshot<MessagePtr> &snapshot);
+
+    /**
+     * @brief Checks the input for tags and registers their corresponding
+     *        predicates.
+     *
+     * @param input the string to check for tags
+     * @return a vector of MessagePredicates requested in the input
+     */
+    static std::vector<std::unique_ptr<MessagePredicate>> parsePredicates(
+        const QString &input);
+
     LimitedQueueSnapshot<MessagePtr> snapshot_;
     QLineEdit *searchInput_{};
     ChannelView *channelView_{};