Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider nicknames when searching for messages #4663

Merged
merged 7 commits into from May 31, 2023

Conversation

chrrs
Copy link
Contributor

@chrrs chrrs commented May 31, 2023

image

With this PR, searching now also finds results using nicknames instead of usernames.

The way I've done this is by prepending the stylized username to the search text, as it seems that nickname matching is based on the stylized username, instead of only the login name / display name. This isn't the best solution, as it does lead to a bit of duplication in the search text for users without a nickname (for example, for the username chrrrs, it would lead to something like chrrrs chrrrs: message). If anyone has a better solution here, I'd be up for changing things around.

@Felanbird
Copy link
Collaborator

layout.emplace<QLabel>(
"Nicknames do not work with features such as search or user highlights."
"\nWith those features you will still need to use the user's original "
"name.");

This will need to be updated also

@chrrs
Copy link
Contributor Author

chrrs commented May 31, 2023

Ah yeah, I updated it now to remove the search part from the text

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

CHANGELOG.md Outdated Show resolved Hide resolved
@pajlada pajlada changed the title Consider nicknames when searching Consider nicknames when searching for messages May 31, 2023
@pajlada pajlada enabled auto-merge (squash) May 31, 2023 18:09
@pajlada
Copy link
Member

pajlada commented May 31, 2023

@chrrs As a first-time contributor, you can add yourself to the contributors list that's shown inside the About page in Chatterino.

If you want this, wait for this PR to be merged in, then open a new PR where you modify the resources/contributors.txt file and add yourself to the list. Make sure to read the comments at the top for instructions.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -64,7 +64,11 @@ MessagePtr IrcMessageBuilder::build()
// message
this->addIrcMessageText(this->originalMessage_);

this->message().searchText = this->message().localizedName + " " +
QString stylizedUsername =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'stylizedUsername' of type 'QString' can be declared 'const' [misc-const-correctness]

Suggested change
QString stylizedUsername =
QString const stylizedUsername =

@@ -64,7 +64,11 @@
// message
this->addIrcMessageText(this->originalMessage_);

this->message().searchText = this->message().localizedName + " " +
QString stylizedUsername =
this->stylizeUsername(this->userName, this->message());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: static member accessed through instance [readability-static-accessed-through-instance]

Suggested change
this->stylizeUsername(this->userName, this->message());
chatterino::IrcMessageBuilder::stylizeUsername(this->userName, this->message());

@@ -294,8 +294,12 @@ MessagePtr TwitchMessageBuilder::build()

this->addWords(splits, twitchEmotes);

QString stylizedUsername =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'stylizedUsername' of type 'QString' can be declared 'const' [misc-const-correctness]

Suggested change
QString stylizedUsername =
QString const stylizedUsername =

@@ -294,8 +294,12 @@

this->addWords(splits, twitchEmotes);

QString stylizedUsername =
this->stylizeUsername(this->userName, this->message());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: static member accessed through instance [readability-static-accessed-through-instance]

Suggested change
this->stylizeUsername(this->userName, this->message());
chatterino::TwitchMessageBuilder::stylizeUsername(this->userName, this->message());

@@ -81,6 +81,22 @@ bool ConcurrentSettings::isMutedChannel(const QString &channelName)
return false;
}

boost::optional<QString> ConcurrentSettings::matchNickname(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'matchNickname' can be made static [readability-convert-member-functions-to-static]

src/singletons/Settings.hpp:49:

-     boost::optional<QString> matchNickname(const QString &username);
+     static boost::optional<QString> matchNickname(const QString &username);

@pajlada pajlada merged commit bf4148a into Chatterino:master May 31, 2023
14 checks passed
@chrrs chrrs deleted the search-nicknames branch June 2, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants