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

Added formatting conversions to messages #97

Closed
wants to merge 3 commits into from

Conversation

Kurocon
Copy link
Contributor

@Kurocon Kurocon commented Aug 4, 2016

I added text formatting conversions to messages. Things like bold, italic and underscored text will be converted into their counterparts on the other service, and colors and strikethroughs will be stripped because they are unsupported on the other platform.

This would fix number 2 from issue #86 and issue #89.

For example on discord:

discord-irc_formatting1

And on IRC:

discord-irc_formatting2

@ekmartin
Copy link
Member

ekmartin commented Aug 7, 2016

From a quick glance this looks really cool, thanks! Will do a more thorough review as fast as I have time :)

export function formatFromDiscordToIRC(text) {
// Apply formatting rules, from most specific to least specific.
return text.replace(/~~(.*?)~~/g, '$1') // Remove unsupported strikethrough
.replace(/__\*\*\*(.*?)\*\*\*__/g, '\u001F\u001D\u0002$1\u000F') // Apply underline-italics-bold
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move \u0001F and similar to a constant? I.e.

const ircFormatCharacters = {
  underline: '\u0001F',
  // and so on
};

@Kurocon
Copy link
Contributor Author

Kurocon commented Aug 14, 2016

Thanks for the comments, you make good points. I'll try to address them as soon as I can. I don't have a lot of experience coding in Javascript, most of my time is spent in Python or Java, so I'm not really familiar with the specifics of the packages, especially the testing framework you're using. Nonetheless, I'll try to fix the issues soon!

@ekmartin
Copy link
Member

No problem, was just minor stuff :)

@ekmartin
Copy link
Member

ekmartin commented Oct 8, 2016

Closing this due to inactivity. Feel free to reopen if you get time to fix the proposed changes!

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