cinchrb / cinch

The IRC Bot Building Framework
http://www.rubydoc.info/gems/cinch
MIT License
1k stars 180 forks source link

Remove spurious NOTICE event addition to event list #80

Closed kyrylo closed 12 years ago

kyrylo commented 12 years ago

I stumbled upon the situation, when I want to react_on NOTICE event. I wrote some code for a plugin and it turned out, that the method, which "reacts on" the event was executing twice:

[2012/05/03 18:48:50.981] >> :Q!TheQBot@CServe.quakenet.org NOTICE Artaius_ :CHALLENGE 34f6f5232658b9facff0aadc3e37954b HMAC-MD5 HMAC-SHA-1 HMAC-SHA-256 LEGACY-MD5
[2012/05/03 18:48:50.982] !! [New thread] For #<Cinch::Handler @event=:notice pattern=#<Cinch::Pattern:0xa4a0970 @prefix=nil, @pattern=/^CHALLENGE (.+?) (.+)$/, @suffix=nil>>: #<Thread:0xa305908> -- 2 in total.
[2012/05/03 18:48:50.982] !! [Artaius::Plugins::Identify] Received challenge '34f6f5232658b9facff0aadc3e37954b'
[2012/05/03 18:48:50.983] !! [Thread done] For #<Cinch::Handler @event=:notice pattern=#<Cinch::Pattern:0xa4a0970 @prefix=nil, @pattern=/^CHALLENGE (.+?) (.+)$/, @suffix=nil>>: #<Thread:0xa305908> -- 1 remaining.
[2012/05/03 18:48:50.983] !! [New thread] For #<Cinch::Handler @event=:notice pattern=#<Cinch::Pattern:0xa4a0970 @prefix=nil, @pattern=/^CHALLENGE (.+?) (.+)$/, @suffix=nil>>: #<Thread:0xa305020> -- 2 in total.
[2012/05/03 18:48:50.983] !! [Artaius::Plugins::Identify] Received challenge '34f6f5232658b9facff0aadc3e37954b'
[2012/05/03 18:48:50.983] << PRIVMSG Q@CServe.quakenet.org :CHALLENGEAUTH artaius efce18a14b1b57ac79015e5da1f57da3d904e1fe17eedfe84a90a51ec9a813b6 HMAC-SHA-256
[2012/05/03 18:48:50.984] << PRIVMSG Q@CServe.quakenet.org :CHALLENGEAUTH artaius efce18a14b1b57ac79015e5da1f57da3d904e1fe17eedfe84a90a51ec9a813b6 HMAC-SHA-256
[2012/05/03 18:48:50.984] !! [Thread done] For #<Cinch::Handler @event=:notice pattern=#<Cinch::Pattern:0xa4a0970 @prefix=nil, @pattern=/^CHALLENGE (.+?) (.+)$/, @suffix=nil>>: #<Thread:0xa305020> -- 0 remaining.
[2012/05/03 18:48:52.991] >> :Q!TheQBot@CServe.quakenet.org NOTICE Artaius_ :You are now logged in as Artaius.
[2012/05/03 18:48:52.992] >> :Q!TheQBot@CServe.quakenet.org NOTICE Artaius_ :Remember: NO-ONE from QuakeNet will ever ask for your password.  NEVER send your password to ANYONE except Q@CServe.quakenet.org.
[2012/05/03 18:48:54.960] >> :Q!TheQBot@CServe.quakenet.org NOTICE Artaius_ :CHALLENGEAUTH is not available once you have authed

So, for every new NOTICE Cinch creates two :notice events to be called by handlers.

The first addition is here: https://github.com/cinchrb/cinch/blob/b73c62cbd3a54ef83121691e57368ba10da2cdf8/lib/cinch/irc.rb#L248 And the second is here: https://github.com/cinchrb/cinch/blob/b73c62cbd3a54ef83121691e57368ba10da2cdf8/lib/cinch/irc.rb#L263

So, whenever it parses such a string as this…:

[2012/05/03 18:48:50.981] >> :Q!TheQBot@CServe.quakenet.org NOTICE Artaius_ :CHALLENGE 34f6f5232658b9facff0aadc3e37954b HMAC-MD5 HMAC-SHA-1 HMAC-SHA-256 LEGACY-MD5

…it molds the following event Array:

events
# => [[:catchall], [:private], [:notice], [:notice]]

For example, for PRIVMSG it would look like this, which is OK:

msg.command
# => "PRIVMSG"
events
# => [[:catchall], [:private], [:message], [:privmsg]]

In this pull request I tried to eliminate this duplication of "notices". Hard to say, whether I fucked up something or not, but at least you are aware of the bug now :)

dominikh commented 12 years ago

Thanks. I had to cherry-pick (instead of merge) your fix though because bug fixes like that have to be applied against the maint branch, which doesn't contain any of the new stuff that gets added to master. The new commit is 889e811.