Highlight notifications for /me actions.
#674
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
englut
force-pushed
the
fix/highlight-notifications
branch
from
6dbe956 to
048fad4
Compare
englut
changed the title
/me actions.
englut
force-pushed
the
fix/highlight-notifications
branch
from
048fad4 to
76f49d4
Compare
casperstorm
reviewed
Dec 16, 2024
src/buffer/channel.rs
Outdated
Comment on lines
217
to
225
| let text_container = | ||
| container(message_content).style(move |theme| match user { | ||
| Some(user) => match our_nick { | ||
| Some(nick) | ||
| if message::references_user( | ||
| user.nickname(), | ||
| nick, | ||
| message, | ||
| ) => | ||
| { | ||
| theme::container::highlight(theme) | ||
| } | ||
| _ => Default::default(), | ||
| }, | ||
| _ => Default::default(), | ||
| }); | ||
|
|
There was a problem hiding this comment.
This could be simplified a bit, not tested but something like:
let text_container = container(message_content).style(|theme| {
if let (Some(user), Some(nick)) = (user, our_nick) {
if message::references_user(user.nickname(), nick, message) {
return theme::container::highlight(theme);
}
}
Default::default()
});There was a problem hiding this comment.
Ah! That's so much nicer. I didn't know that you could combine the Some(..) assignment that way. I did think that there was something to optimize there! Thanks for this 🙏
The simplification has been tested and pushed!
englut
force-pushed
the
fix/highlight-notifications
branch
from
bd8af8d to
3506255
Compare
englut
force-pushed
the
fix/highlight-notifications
branch
from
3506255 to
3d97d05
Compare
|
I noticed that highlights in query weren't present. I've added this to the PR. |
englut
force-pushed
the
fix/highlight-notifications
branch
from
903df07 to
41d2e61
Compare
casperstorm
approved these changes
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found bugs with highlighting notifications when someone mentions you with a
/meaction.There's three related issues:
/meaction highlights are not triggered./meaction is not visually highlighted./meaction highlights not showing in Highlights buffer.Above issues should be addressed with this PR.