From 720e5aa25f67c45b2c97304dedf7914f096306d8 Mon Sep 17 00:00:00 2001 From: Leon Richardt Date: Mon, 9 Sep 2019 15:21:49 +0200 Subject: [PATCH] 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 | 7 ++ src/messages/search/AuthorPredicate.cpp | 24 ++++++ src/messages/search/AuthorPredicate.hpp | 38 +++++++++ src/messages/search/LinkPredicate.cpp | 22 ++++++ src/messages/search/LinkPredicate.hpp | 26 +++++++ src/messages/search/MessagePredicate.hpp | 31 ++++++++ src/messages/search/SubstringPredicate.cpp | 15 ++++ src/messages/search/SubstringPredicate.hpp | 41 ++++++++++ src/providers/twitch/TwitchAccount.cpp | 18 ++--- src/providers/twitch/TwitchApi.cpp | 4 +- src/providers/twitch/TwitchBadges.cpp | 2 +- src/singletons/Toasts.cpp | 2 +- src/widgets/dialogs/LogsPopup.cpp | 3 +- src/widgets/helper/SearchPopup.cpp | 91 +++++++++++++++++++--- src/widgets/helper/SearchPopup.hpp | 25 ++++++ 15 files changed, 323 insertions(+), 26 deletions(-) create mode 100644 src/messages/search/AuthorPredicate.cpp create mode 100644 src/messages/search/AuthorPredicate.hpp create mode 100644 src/messages/search/LinkPredicate.cpp create mode 100644 src/messages/search/LinkPredicate.hpp create mode 100644 src/messages/search/MessagePredicate.hpp create mode 100644 src/messages/search/SubstringPredicate.cpp create mode 100644 src/messages/search/SubstringPredicate.hpp diff --git a/chatterino.pro b/chatterino.pro index 0c38c50a7..c205978e8 100644 --- a/chatterino.pro +++ b/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 \ diff --git a/src/messages/search/AuthorPredicate.cpp b/src/messages/search/AuthorPredicate.cpp new file mode 100644 index 000000000..8e1b73280 --- /dev/null +++ b/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 diff --git a/src/messages/search/AuthorPredicate.hpp b/src/messages/search/AuthorPredicate.hpp new file mode 100644 index 000000000..8b3bbced2 --- /dev/null +++ b/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 diff --git a/src/messages/search/LinkPredicate.cpp b/src/messages/search/LinkPredicate.cpp new file mode 100644 index 000000000..41fec567b --- /dev/null +++ b/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 diff --git a/src/messages/search/LinkPredicate.hpp b/src/messages/search/LinkPredicate.hpp new file mode 100644 index 000000000..4d73920b5 --- /dev/null +++ b/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 diff --git a/src/messages/search/MessagePredicate.hpp b/src/messages/search/MessagePredicate.hpp new file mode 100644 index 000000000..d74557d15 --- /dev/null +++ b/src/messages/search/MessagePredicate.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include "messages/Message.hpp" + +#include + +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 diff --git a/src/messages/search/SubstringPredicate.cpp b/src/messages/search/SubstringPredicate.cpp new file mode 100644 index 000000000..4a715f7be --- /dev/null +++ b/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 diff --git a/src/messages/search/SubstringPredicate.hpp b/src/messages/search/SubstringPredicate.hpp new file mode 100644 index 000000000..31f43102e --- /dev/null +++ b/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 diff --git a/src/providers/twitch/TwitchAccount.cpp b/src/providers/twitch/TwitchAccount.cpp index 2f1255a65..3659edb67 100644 --- a/src/providers/twitch/TwitchAccount.cpp +++ b/src/providers/twitch/TwitchAccount.cpp @@ -95,7 +95,7 @@ void TwitchAccount::loadIgnores() "/blocks"); NetworkRequest(url) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onSuccess([=](auto result) -> Outcome { auto document = result.parseRapidJson(); @@ -168,7 +168,7 @@ void TwitchAccount::ignoreByID( "/blocks/" + targetUserID); NetworkRequest(url, NetworkRequestType::Put) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { onFinished(IgnoreResult_Failed, @@ -245,7 +245,7 @@ void TwitchAccount::unignoreByID( "/blocks/" + targetUserID); NetworkRequest(url, NetworkRequestType::Delete) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { onFinished( @@ -279,7 +279,7 @@ void TwitchAccount::checkFollow(const QString targetUserID, "/follows/channels/" + targetUserID); NetworkRequest(url) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { if (errorCode == 203) @@ -308,7 +308,7 @@ void TwitchAccount::followUser(const QString userID, "/follows/channels/" + userID); NetworkRequest(requestUrl, NetworkRequestType::Put) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onSuccess([successCallback](auto result) -> Outcome { // TODO: Properly check result of follow request @@ -326,7 +326,7 @@ void TwitchAccount::unfollowUser(const QString userID, "/follows/channels/" + userID); NetworkRequest(requestUrl, NetworkRequestType::Delete) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([successCallback](int code) { if (code >= 200 && code <= 299) @@ -368,7 +368,7 @@ void TwitchAccount::loadEmotes() "/emotes"); NetworkRequest(url) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { log("[TwitchAccount::loadEmotes] Error {}", errorCode); @@ -409,7 +409,7 @@ void TwitchAccount::autoModAllow(const QString msgID) .header("Content-Type", "application/json") .header("Content-Length", QByteArray::number(qba.size())) .payload(qba) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { log("[TwitchAccounts::autoModAllow] Error {}", errorCode); @@ -429,7 +429,7 @@ void TwitchAccount::autoModDeny(const QString msgID) .header("Content-Type", "application/json") .header("Content-Length", QByteArray::number(qba.size())) .payload(qba) - + .authorizeTwitchV5(this->getOAuthClient(), this->getOAuthToken()) .onError([=](int errorCode) { log("[TwitchAccounts::autoModDeny] Error {}", errorCode); diff --git a/src/providers/twitch/TwitchApi.cpp b/src/providers/twitch/TwitchApi.cpp index 957e91323..d91fc3701 100644 --- a/src/providers/twitch/TwitchApi.cpp +++ b/src/providers/twitch/TwitchApi.cpp @@ -16,7 +16,7 @@ void TwitchApi::findUserId(const QString user, QString requestUrl("https://api.twitch.tv/kraken/users?login=" + user); NetworkRequest(requestUrl) - + .authorizeTwitchV5(getDefaultClientID()) .timeout(30000) .onSuccess([successCallback](auto result) mutable -> Outcome { @@ -65,7 +65,7 @@ void TwitchApi::findUserName(const QString userid, QString requestUrl("https://api.twitch.tv/kraken/users/" + userid); NetworkRequest(requestUrl) - + .authorizeTwitchV5(getDefaultClientID()) .timeout(30000) .onSuccess([successCallback](auto result) mutable -> Outcome { diff --git a/src/providers/twitch/TwitchBadges.cpp b/src/providers/twitch/TwitchBadges.cpp index 7fecbc91f..84d733115 100644 --- a/src/providers/twitch/TwitchBadges.cpp +++ b/src/providers/twitch/TwitchBadges.cpp @@ -18,7 +18,7 @@ void TwitchBadges::loadTwitchBadges() "https://badges.twitch.tv/v1/badges/global/display?language=en"); NetworkRequest(url) - + .onSuccess([this](auto result) -> Outcome { auto root = result.parseJson(); auto badgeSets = this->badgeSets_.access(); diff --git a/src/singletons/Toasts.cpp b/src/singletons/Toasts.cpp index 94e12a064..c72b43e58 100644 --- a/src/singletons/Toasts.cpp +++ b/src/singletons/Toasts.cpp @@ -214,7 +214,7 @@ void Toasts::fetchChannelAvatar(const QString channelName, channelName); NetworkRequest(requestUrl) - + .authorizeTwitchV5(getDefaultClientID()) .timeout(30000) .onSuccess([successCallback](auto result) mutable -> Outcome { diff --git a/src/widgets/dialogs/LogsPopup.cpp b/src/widgets/dialogs/LogsPopup.cpp index c9b8b0599..0333fbb28 100644 --- a/src/widgets/dialogs/LogsPopup.cpp +++ b/src/widgets/dialogs/LogsPopup.cpp @@ -175,7 +175,8 @@ void LogsPopup::getOverrustleLogs() MessageColor::System); builder.emplace(text, MessageElementFlag::Text, MessageColor::Text); - builder.message().searchText = text; + builder.message().messageText = text; + builder.message().displayName = this->userName_; messages.push_back(builder.release()); } } diff --git a/src/widgets/helper/SearchPopup.cpp b/src/widgets/helper/SearchPopup.cpp index e799cef6f..e656bc5e9 100644 --- a/src/widgets/helper/SearchPopup.cpp +++ b/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 &snapshot) + +ChannelPtr SearchPopup::filter(const QString &text, const QString &channelName, + const LimitedQueueSnapshot &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> SearchPopup::parsePredicates( + const QString &input) +{ + static QRegularExpression predicateRegex(R"(^(\w+):([\w,]+)$)"); + + auto predicates = std::vector>(); + 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()); + } + else + { + remove = false; + } + + // remove or advance + it = remove ? words.erase(it) : ++it; + } + else + { + ++it; + } + } + + if (!authors.empty()) + predicates.push_back(std::make_unique(authors)); + + if (!words.empty()) + predicates.push_back( + std::make_unique(words.join(" "))); + + return predicates; +} + } // namespace chatterino diff --git a/src/widgets/helper/SearchPopup.hpp b/src/widgets/helper/SearchPopup.hpp index 05a795054..261886c18 100644 --- a/src/widgets/helper/SearchPopup.hpp +++ b/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 @@ -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 &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> parsePredicates( + const QString &input); + LimitedQueueSnapshot snapshot_; QLineEdit *searchInput_{}; ChannelView *channelView_{};