search

martynsmith / node-irc

NodeJS IRC client library
GNU General Public License v3.0
1.33k stars 424 forks source link

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

Open BlayTheNinth opened 8 years ago

BlayTheNinth commented 8 years ago

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

Trinitas commented 8 years ago

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 commented 8 years ago

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 https://github.com/martynsmith/node-irc/issues/410#issuecomment-176542833.

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

jirwin commented 8 years ago

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 commented 8 years ago

The twitch pull request, https://github.com/martynsmith/node-irc/pull/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 commented 7 years ago

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?)