cinchrb / cinch

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

Detect differences in nickname between our copy and a 311 WHOIS reply #165

Closed nickrw closed 6 years ago

nickrw commented 10 years ago

User#nick always returns a nick in the same case as the first time the bot saw that user online, because it is cached, case insensitive, in the UserList.

To reproduce, use something like the following:

match /echoname ([^ ]+)$/, :method => :echoname
def echoname(m, user)
  uobj = User(user)
  m.reply(uobj.nick)
end

The attached patch detects differences between User#nick and the nickname returned by a 311 WHOIS reply message and calls User#update_nick if necessary.

dominikh commented 10 years ago

There is at least one more case in which the casing will not match, when doing a User("foo") before joining a channel with someone called FOO in it. So we should probably at least also update the nick in on_354 in irc.rb. Hopefully that's the only other place we need to cover.

todd-a-jacobs commented 8 years ago

I just got bitten by this issue, which is impacting accurate keys when serializing to YAML::Store. This issue has been open for a year or more. Any chance of this patch, or a similar patch, being merged?

dominikh commented 8 years ago

If someone updates the patch to fix all of the places, not just one of them, yes.

todd-a-jacobs commented 8 years ago

@dominikh I'm not all that familiar with the code base. Is the issue just that you also need the guts of the patch duplicated to #on_354 as well as #on_311? I might take a stab at it, but I don't really grok enough of the code to offer a cleaner solution offhand.

Perhaps more importantly, I've asked in the cinchrb IRC channel before about how to test cinch without an actual IRC server, and was told that there currently isn't a mechanism for this. I'm not really sure how to simulate the IRC messages well enough to unit test the proposed solutions, so it feels a bit like performing surgery in the dark.

I'm always happy to lend a hand, but want to ensure I'm approaching a patch in a way that is likely to be accepted and testable. What do you advise?

dominikh commented 8 years ago

Is the issue just that you also need the guts of the patch duplicated to #on_354 as well as #on_311?

Yes. Additionally, one would have to verify that these are the only two places this checks needs to be.

I'm not really sure how to simulate the IRC messages well enough to unit test the proposed solutions, so it feels a bit like performing surgery in the dark.

Certain design flaws in Cinch make unit testing a real pain, so it's not something that's generally done.