Refactor emote parsing, fix some emote display issues #529
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
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #529 +/- ##
=========================================
+ Coverage 7.32% 15.72% +8.39%
=========================================
Files 39 40 +1
Lines 4161 4514 +353
=========================================
+ Hits 305 710 +405
+ Misses 3856 3804 -52 ☔ View full report in Codecov by Sentry. |
|
Just confirming so I don't accidently nuke your other branch - you'd like to get this merged before #524, yeah? |
|
Yes, I still need to clean up and split into different commits the other branch. |
|
I believe I'll be able to go over this code in the next couple of days. |
- Split the to_vec::highlight function into get_emote_span, highlight, and build_line - Fix bug where username highlights was using byte indices, and search highlight was using char indices. - Don't format and clone time_sent string if config.show_datetimes is false - Fix crash when running with both show_datetimes and username_shown set to false - Fix right_align_usernames offset with emojis in username, fix overflow when usernames were > NAME_MAX_CHARACTERS len - Fix emotes smaller than one cell being skipped when trying to display them - Move some emote related functions into utils/ - Add some test for build_line and related functions
Nogesma
force-pushed
the
refactor/emote-parsing
branch
from
3bbd79f to
7ea1157
Compare
I found out kitty does not need to have the DIACRITICS_ZERO specified, and we can avoid it entirely. This also removes the Spans(ZERO_WIDTH_SPACE) between emotes/text, as they are not needed, and were causing rendering issues with ratatui.
Xithrius
approved these changes
Feb 7, 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.
While cleaning up my code for the emote picker, I ended up rewriting most of the emote parsing code.
To avoid making the other PR even bigger, here's a separate one.
Most important bug fixes are the emote overlays that should now be properly centered on top of the base emote, and emotes smaller than the width of 1 terminal cell were not being displayed.
There is a more complete list of fixes in my commits messages.
I also added a bunch of tests for the conversion from &str to vec![Span].
A lot of those additions are from adding tests, so hopefully this is not too large of a PR to review.
Let me know if you find any issues, or need any changes.