cinchrb / cinch

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

User.mask() fails for custom mask string #94

Open blowback opened 12 years ago

blowback commented 12 years ago

The regex pattern in Mask.new() expects the general form nick!user@host. User.mask() however allows you to specify a custom printf-format style string, in which any or all of [nick, user, mask] may be elided - it then calls Mask.new() which fails.

e.g.

mask = User.mask.new("%u@%h")

lib/cinch/mask.rb:17:in `initialize': undefined method `[]' for nil:NilClass (NoMethodError)

I have changed the regex in Mask.new to:

@nick, @user, @host = mask.match(/(?:(.+)!)?(?:(.+)@)?(.+)/)[1..-1]

However, this whole thing probably needs rethinking.

I would suggest that User.mask() take no args and return a Mask object (in canonical n!u@h form), and that User.some_other_method(mask="%n!%u@%h") return a String, since if you have customised the mask string, you can't really expect it to participate in Mask operations.

dominikh commented 12 years ago

This will require some thinking, but generally you are right. At first I was going to say that you are expected to not leave out fields in the mask specification you pass to User#mask, but that makes no difference considering that %a (authname) is something that would never be part of a mask as IRC understands it.

So yeah, I'll have to think of a better method name. Keep in mind though that I cannot fundamentally change how User#mask works until I release Cinch 3.0 (which is not planned at all yet), because that would break backwards compatibility. So all cases that currently do not raise an exception have to continue to work.

blowback commented 12 years ago

Yes, tricky one, but currently anyone calling mask() with a custom format string is going to get an error anyway unless they have the magic n!u@h pattern in there somewhere.

Maybe prepend it to the user string if the user string doesn't already start with that? Or retry with the standard string if Mask.new fails? Or raise an exception that explains matters?

dominikh commented 12 years ago

By no means does it have to include %n, %u and %h. "*!*@*" would be just as valid. It just has to match something!something@something.

blowback commented 12 years ago

Yes, I know, that's why I said 'n' and not '%n'.

dominikh commented 11 years ago

The rational behind allowing custom masks was to be able to generate masks such as *!*@something, not to completely leave out fields.

Why exactly we allow fields such as auth name and real name eludes me right now, though…