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

Strip IRC formatting before sending to Discord #166

Closed
wants to merge 1 commit into from
Closed

Strip IRC formatting before sending to Discord #166

wants to merge 1 commit into from

Conversation

rossengeorgiev
Copy link

No description provided.

@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Coverage increased (+0.01%) to 97.945% when pulling 6bb4b2c on rossengeorgiev:patch-1 into ad2c974 on reactiflux:master.

@ekmartin
Copy link
Member

ekmartin commented Dec 1, 2016

Maybe a better idea would be to convert the formatting to their Discord counter part, and vice versa? This was implemented by @Kurocon in #97, but the PR was never completed. It's a good implementation though, just needed some finishing touches. If you have time I'm sure he wouldn't mind if you extended his PR and fixed the relevant feedback.

@rossengeorgiev
Copy link
Author

rossengeorgiev commented Dec 1, 2016

I can't remember seeing anyone ever use underline, italic or bold on IRC for all the years. It is a nice idea, but I don't see any use for it. I am not interested in working on that PR. Just want clean readable text.

If that PR ever gets touched, it can easily remove the 2 lines from this one.

@ekmartin
Copy link
Member

ekmartin commented Dec 1, 2016

That's alright, I could agree with merging this in the meantime. Maybe use an existing library for it instead though? E.g. irc-colors.js has a stripColorsAndStylefunction (which seems to have tests as well).

@rossengeorgiev
Copy link
Author

Why add a dependency for something so simple?

@ekmartin
Copy link
Member

ekmartin commented Dec 9, 2016

@rossengeorgiev: Because it adds tests for that specific function as well. Regarding the small module discussion there's plenty of arguments here: sindresorhus/ama#10 (comment)

I don't really think this is the same discussion as that though, as the function you're adding here really isn't that simple - if you're not overly familiar with IRC formatting escape sequences. That's fine though, as long as it's tested, which in your case it's not.

@StrikerMan780
Copy link

"I don't see any use for it."

I can. I run an IRC bot that gets the output of a game server's chat, and it color-codes the player names based on their team. It also converts the game's chat color codes to IRC ones.

@ekmartin
Copy link
Member

@StrikerMan780: I'm open to both implementations, but I'm going to close this PR due to inactivity. If anyone wants to give it a go, feel free to read the comments above and send a new PR.

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

4 participants