sheriffbot should also be addressable with a comma in addition to colon
Created attachment 131836 [details] Patch
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.
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 ?
Created attachment 131941 [details] Patch
(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!
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. :)
Committed r110837: <http://trac.webkit.org/changeset/110837>