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

Support displaying shortcodes instead of Emojis in messages #222

Merged
merged 5 commits into from Mar 23, 2024

Conversation

bbliem
Copy link
Contributor

@bbliem bbliem commented Mar 22, 2024

The font in my terminal doesn't support most emoji characters and shows ugly placeholders instead. For reactions, there is the setting reaction_shortcode_display that allows me to see human-readable shortcodes instead of the emojis. However, for emojis in messages there is no such feature.

I added a new setting message_shortcode_display that controls whether shortcodes are displayed instead of emojis in messages.

Replacing emojis with shortcodes happens in TextPrinter (or, for plain text, in Message::show_msg()).

I hope the changes are alright. I'd be happy to improve the code further and I'd be grateful for some feedback as I'm not very experienced in Rust yet.

@ulyssa
Copy link
Owner

ulyssa commented Mar 23, 2024

This is a good idea! Thank you for putting this together!

I'd be happy to improve the code further and I'd be grateful for some feedback as I'm not very experienced in Rust yet.

It looks like you've already figured it out, but several things worth mentioning are:

  • cargo +nightly fmt will take care of reformatting to the code
  • cargo clippy will do linting and other checks; it's especially useful for getting simpler or improved alternatives to how you wrote something.
  • cargo test will run the test suite

The code looks good. One thing that I think would be good to add to the PR is to add tests to test_emoji_shortcodes of multi-codepoint Emojis, like:

  • 🐻‍❄️ to :polar_bear: (bear + ZWJ + snowflake + variation selector 16)
  • 🇨🇦 to :canada: (regional indicator letters C and A)

You're already using the .graphemes iterator, so I'm pretty sure testing those Emojis should just work without any other changes needed.

@ulyssa ulyssa added this to the v0.0.9 milestone Mar 23, 2024
@bbliem
Copy link
Contributor Author

bbliem commented Mar 23, 2024

Thanks for the hints! And good idea about testing more complex emojis. Done!

@ulyssa ulyssa changed the title Display shortcodes instead of emojis in messages Support displaying shortcodes instead of emojis in messages Mar 23, 2024
@ulyssa ulyssa merged commit 23a729e into ulyssa:main Mar 23, 2024
3 checks passed
@ulyssa ulyssa changed the title Support displaying shortcodes instead of emojis in messages Support displaying shortcodes instead of Emojis in messages Mar 23, 2024
@bbliem bbliem deleted the message-emoji-shortcodes branch March 24, 2024 09:14
@ulyssa ulyssa mentioned this pull request Mar 29, 2024
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

2 participants