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

Translate custom emoji references from IRC to Discord #256

Merged
merged 1 commit into from
Jul 1, 2017

Conversation

Throne3d
Copy link
Collaborator

@Throne3d Throne3d commented Jun 29, 2017

When an IRC user uses a custom emoji reference (e.g. :testemoji:), it should be translated into the actual
emoji for Discord users, instead of appearing in plain text.

This also adds tests to ensure other emoji-like phrases do not get converted or cause the bot to crash.

(Discord users enter something such as :testemoji:, which gets converted into e.g. <:testemoji:987654321> – we already translate this to IRC, stripping the angle brackets and extra digits, but don't translate the other way as I would expect.)

Addresses issue #250.

When an IRC user uses a custom emoji reference (e.g.
:testemoji:), it should be translated into the actual
emoji for Discord users, instead of appearing in plain
text.

This also adds tests to ensure other emoji-like phrases
do not get converted or cause the bot to crash.
@Throne3d Throne3d self-assigned this Jun 29, 2017
@coveralls
Copy link

coveralls commented Jun 29, 2017

Coverage Status

Coverage increased (+0.03%) to 98.485% when pulling b9ff8b4 on Throne3d:add/emoji-mentions into 80c3752 on reactiflux:master.

@Throne3d
Copy link
Collaborator Author

Throne3d commented Jun 29, 2017

As a note: I think all server-specific emoji have requiresColons true, but in the case that one doesn't, it presumably shouldn't map :emojiname:, since the emoji doesn't "require colons". I'm not sure what it would look like, as all the server-specific emoji I can find or create seem to need colons, but I thought it best to require that anyway.

@ekmartin ekmartin merged commit 474d298 into reactiflux:master Jul 1, 2017
@ekmartin
Copy link
Member

ekmartin commented Jul 1, 2017

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants