Bug 81101 – sheriffbot should also be addressable with a comma in addition to colon

• Component: New Bugs
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

sheriffbot should also be addressable with a comma in addition to colon

[Gustavo Noronha (kov)]

Created attachment 131836 [details]
Patch

[Gustavo Noronha (kov)]

The IRC client I use adds a comma after the nickname, when tab-completing, instead of a :. sheriffbot ignores those messages, would be good to allow both.

[Adam Barth]

Comment on attachment 131836 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131836&action=review

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:79
> +        if not irclib.irc_lower(request[:nickname_length]).startswith(irclib.irc_lower(self.connection.get_nickname())):

What's the point of trimming request to nickname_length if you're going to use startswith anyway?

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:84
> +        if request[nickname_length] == ':':

Won't this crash if len(request) == nickname_length ?

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:85
> +            request = request.split(':')

Also, don't we need to use split(":", 1) to avoid breaking in cases like:

sheriffbot: some message with a : character

?

[Gustavo Noronha (kov)]

Created attachment 131941 [details]
Patch

[Gustavo Noronha (kov)]

(In reply to comment #3)
> > Tools/Scripts/webkitpy/common/net/irc/ircbot.py:79
> > +        if not irclib.irc_lower(request[:nickname_length]).startswith(irclib.irc_lower(self.connection.get_nickname())):
> 
> What's the point of trimming request to nickname_length if you're going to use startswith anyway?

The idea was to only apply irc_lower on what was needed for the test, but it's probably a worthless micro-optimization anyway.

> > Tools/Scripts/webkitpy/common/net/irc/ircbot.py:85
> > +            request = request.split(':')
> 
> Also, don't we need to use split(":", 1) to avoid breaking in cases like:
> 
> sheriffbot: some message with a : character

And the original code even did it, properly =). Thanks for the review!

[Adam Barth]

Comment on attachment 131941 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131941&action=review

> Tools/Scripts/webkitpy/common/net/irc/ircbot.py:86
> +        # Some IRC clients, like xchat-gnome, default to using a comma
> +        # when addressing someone.
> +        try:
> +            vocative_separator = request[len(connection.get_nickname())]
> +        except IndexError:
> +            return

This patch is fine, but you don't really need to use a try-catch block here.  A simple length test should work fine.  :)

[Gustavo Noronha (kov)]

Committed r110837: <http://trac.webkit.org/changeset/110837>