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

Implement irc-message and irc-prefix-parser modules #447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BlayTheNinth
Copy link

Changed parse_message to use irc-message and irc-prefix-parser modules (adds IRCv3 message tag support).
Updated parse-line text fixtures to include new fields "raw" and "tags".
Added dependencies to irc-message and irc-prefix-parser.
Fixed test-parse-line equal() order (actual is first parameter, expected is second).

It's backwards compatible and adds the two new fields "raw" (string, contains raw IRC message) and "tags" (object, contains IRCv3 message tags) to the message object.

Fixes #410

… (adds IRCv3 message tag support).

Update parse-line text fixtures to include new fields "raw" and "tags".
Add dependencies to irc-message and irc-prefix-parser.
Fix test-parse-line equal() order (actual is first parameter, expected is second).
@Trinitas
Copy link
Contributor

I'm sure there is someone who can implement this new "tag/raw" command without using modules and What do we do if there is a bug in irc-message and the creator abandons the project?

I think I'm not the only one who wants this project "module free"

@BlayTheNinth
Copy link
Author

Sure it could have been re-implemented here, but why reinvent the wheel when there is a seperately tested module designed just for this case? If the unlikely scenario of a bug in irc-message.parse() happens and the project is abandoned (like...this one), it won't be a big deal porting the implementation over. I'm just going off what was said in #410 (comment).

Besides, there's already a dependency on irc-colors. This project is not module free to begin with.

@jirwin
Copy link
Collaborator

jirwin commented May 6, 2016

I'm +1 to this change. I'm not confident enough in the tests to blindly merge it, so I'll run it on my various bots to try it out.

@ghost
Copy link

ghost commented May 13, 2016

The twitch pull request, #425, parses message tags. That could probably be closed with this change. As suggested in that bug, twitch APIs should be implemented in a separate module.

@myndzi
Copy link

myndzi commented Nov 19, 2016

For what it's worth, this patch needs a little extra help. In parse_message, it should be wrapping the prefix stuff in if (message.prefix) or the like, since pings from the server don't have a prefix and thus we can't assume prefix is anything but null.

(or, if we have access to the server we're connected to, we should set that?)

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.

Add IRCv3 tag support to parseMessage
4 participants