search

Xithrius / twitch-tui

Twitch chat in the terminal.
https://xithrius.github.io/twitch-tui/
Apache License 2.0
437 stars 32 forks source link

Fix off by one error in hightlight #650

Closed Nogesma closed 3 weeks ago

Nogesma commented 1 month ago

It's a quick fix but should work. I don't know if it would be more efficient to maybe get the highlights indices after wrapping the message, which is the core of the issue here. edit: I just realized it would break highlight of words spanning multiple lines so I'm not sure what a better solution would look like.

I couldn't manage to find how to send a message with newlines either with the twitch website or with twt so I just added a test case for it, but maybe @vesdev can confirm this fixes it?

Oh and I reverted the PR completely, which downgrades ahash, let me know if you want me to update it back.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 96.96970% with 2 lines in your changes missing coverage. Please review.

Project coverage is 23.10%. Comparing base (62f53e4) to head (cd3570c). Report is 1 commits behind head on main.

Files Patch % Lines
src/handlers/data.rs 96.96% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #650 +/- ## ========================================== + Coverage 17.87% 23.10% +5.22% ========================================== Files 42 42 Lines 5431 5492 +61 ========================================== + Hits 971 1269 +298 + Misses 4460 4223 -237 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vesdev commented 1 month ago

I can still reproduce the issue image

Nogesma commented 1 month ago

Hmm, what client are you using that inserts newlines in the message? So I can test this myself.

Also did you make sure the PR was applied?

vesdev commented 1 month ago

Hmm, what client are you using that inserts newlines in the message? So I can test this myself.

Also did you make sure the PR was applied?

Just twt, there are no newlines. Only line breaks are done by textwrap afaik. And yes the pr is applied

Nogesma commented 1 month ago

Ah my bad then I didn't understand the issue right, let me fix this

Nogesma commented 4 weeks ago

This should now be fixed, let me know if it works!

vesdev commented 4 weeks ago

This looks like the correct fix and works 👍 Still doesn't build without updating ahash, but that could be a separate pr

Xithrius commented 3 weeks ago

Ok this time I'm pretty sure I tested it properly. I'll update the required dependencies in a new commit.

Xithrius commented 3 weeks ago

Available in https://github.com/Xithrius/twitch-tui/releases/tag/v2.6.15