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

Convert text styles between IRC/Discord #205

Merged
merged 8 commits into from
Mar 29, 2017

Conversation

rahatarmanahmed
Copy link
Collaborator

@rahatarmanahmed rahatarmanahmed commented Mar 25, 2017

This is a continuation of #97, but instead of manually parsing the text ourselves, I've opted to use the irc-formatter, irc-colors, and simple-markdown packages to keep this project's code clean. (I think simple-markdown is the parser Discord uses but I assume that only because they have their own fork of it on github.)

image
image

Resolves #195

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 97.688% when pulling 336e87e on rahatarmanahmed:convert-formatting into 26626e5 on reactiflux:master.

@rahatarmanahmed rahatarmanahmed changed the title Convert formatting between IRC/Discord Convert text styles between IRC/Discord Mar 25, 2017
@rahatarmanahmed
Copy link
Collaborator Author

Ah hell, there's some inconsistent conversion from irc->discord if you do some nested styles. irc-formatter isn't really an adequate package since it gives you a flat list of blocks. I'll probably try to write a package to build like a parse tree of irc message formats to fix this.

@rahatarmanahmed
Copy link
Collaborator Author

Nevermind! Took a different approach to IRC->discord formatting with the same package that should have less weird results.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 97.778% when pulling 86fadd5 on rahatarmanahmed:convert-formatting into 26626e5 on reactiflux:master.

@Throne3d
Copy link
Collaborator

Throne3d commented Mar 25, 2017

I don't think this is relevant, but I was poking around at doing a configuration to allow stripping of mIRC formatting and I think I found a bug in how irc-colors.js handles stripColorsAndStyle (see here). In case it's useful or you notice some weird behavior when poking around at things.

@rahatarmanahmed
Copy link
Collaborator Author

Good to know. This PR only uses irc-colors for avoiding hardcoding the style character codes so it shouldn't be affected.

But the funny coincidence is that I was the one who added the stripping functions in irc-colors 😮


function mdNodeToIRC(node) {
let content = node.content;
if (_.isArray(content)) content = content.map(mdNodeToIRC).join('');
Copy link
Member

Choose a reason for hiding this comment

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

Could just use Array.isArray I guess?

export function formatFromIRCToDiscord(text) {
const blocks = ircFormatting.parse(text).map(block =>
// Consider reverse as italic, some IRC clients use that
_.assign({}, block, { italic: block.italic || block.reverse })
Copy link
Member

Choose a reason for hiding this comment

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

The package has a Node >6 requirement anyway, since discord.js enforces that, so could do

{
  ...block,
  { italic: block.italic || block.reverse }
}

@ekmartin
Copy link
Member

Thanks! This seems useful, and the implementation looks clean.

@rahatarmanahmed
Copy link
Collaborator Author

Ah my bad, not sure how I overlooked the babel config and node requirements.

@coveralls
Copy link

coveralls commented Mar 29, 2017

Coverage Status

Coverage increased (+0.5%) to 97.814% when pulling 200a0b9 on rahatarmanahmed:convert-formatting into 26626e5 on reactiflux:master.

@ekmartin ekmartin merged commit 9915db1 into reactiflux:master Mar 29, 2017
@ekmartin
Copy link
Member

I'll cut a release when we've got the last two outstanding PRs merged.
I've added you as a collaborator - feel free to help review PRs if you want to and have time :)

@rahatarmanahmed rahatarmanahmed deleted the convert-formatting branch March 29, 2017 20:30
@ekmartin
Copy link
Member

ekmartin commented Apr 4, 2017

Released in 2.3.0!

@StrikerMan780
Copy link

Got a question about this, is it possible to have IRC colors show in Discord, or is it only formatting like bold and underline?

@rahatarmanahmed
Copy link
Collaborator Author

Only formatting, discord markdown doesn't have any coloring option (except for syntax highlighting)

Comment on lines +27 to +29
for (let i = 0; i <= blocks.length; i += 1) {
// Default to unstyled blocks when index out of range
const block = blocks[i] || {};
Copy link

Choose a reason for hiding this comment

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

When can blocks[i] ever be undefined?

Copy link

Choose a reason for hiding this comment

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

Ah, i <= blocks.length not i < blocks.length. Sneaky code.

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

7 participants