Response to comments.

Author: neiljp

zulip_bots/zulip_bots/bots/wikipedia/wikipedia.py

@@ -21,7 +21,7 @@ class WikipediaHandler(object):
     META = {
         'name': 'Wikipedia',
         'description': 'Searches Wikipedia for a term and returns the top article.',
-        'defaults': False,  # Let bot handle all messages
+        'default_commands_enabled': False,
     }
 
     def usage(self):

zulip_bots/zulip_bots/bots/xkcd/xkcd.py

@@ -19,7 +19,7 @@ class XkcdHandler(object):
     META = {
         'name': 'XKCD',
         'description': 'Fetches comic strips from https://xkcd.com.',
-        'defaults': True,
+        'default_commands_enabled': True,
         'commands': OrderedDict([
                 ('latest', "Show the latest comic strip"),
                 ('random', "Show a random comic strip"),

zulip_bots/zulip_bots/lib.py

@@ -191,34 +191,50 @@ def is_private_message_from_another_user(message_dict, current_user_id):
     return False
 
 def setup_default_commands(bot_details, message_handler):
-    def def_about():
-        if bot_details['description'] == "":
-            return "**{}**".format(bot_details['name'])
-        return "**{}**: {}".format(bot_details['name'], bot_details['description'])
+    def default_empty_response():
+        return "Oops. Your message was empty."
 
-    def def_help():
-        return ("\n".join("**{}** - {}".format(k, v[1])
-                          for k, v in defaults.items() if k) + "\n" +
-                "\n".join("**{}** - {}".format(k, v)
-                          for k, v in bot_details['commands'].items() if k))
+    def default_about_response():
+        if bot_details['description'] == "":
+            return "**{name}**".format(**bot_details)
+        return "**{name}**: {description}".format(**bot_details)
 
-    def def_commands():
+    def default_commands_response():
         return "**Commands**: {} {}".format(
-            " ".join(k for k in defaults if k),
-            " ".join(k for k in bot_details['commands'] if k))
-    defaults = OrderedDict([  # Variable definition required for callbacks above
-        ('', (lambda: "Oops. Your message was empty.", "[BLANK MESSAGE NOT SHOWN]")),
-        ('about', (def_about, "The type and use of this bot")),
+            " ".join(name for name in command_defaults if name),
+            " ".join(name for name in bot_details['commands'] if name))
+
+    def commands_list():
+        return ("\n".join("**{}** - {}".format(name, options[1])
+                          for name, options in command_defaults.items() if name) +
+                "\n" +
+                "\n".join("**{}** - {}".format(name, description)
+                          for name, description in bot_details['commands'].items() if name))
+
+    def default_help_response():
+        return "{}\n{}\n{}".format(default_about_response(),
+                                   message_handler.usage(), commands_list())
+        # FIXME Add boilerplate text above, to include eg. how to mention bot?
+        # Should usage be included?
+
+    command_defaults = OrderedDict([  # Variable definition required for callbacks above
+        ('', (default_empty_response, "[BLANK MESSAGE NOT SHOWN]")),
+        ('about', (default_about_response, "The type and use of this bot")),
         ('usage', ((lambda: message_handler.usage(), "Bot-provided usage text"))),
-        ('help', (lambda: "{}\n{}\n{}".format(def_about(), message_handler.usage(), def_help()),
-                  "This help text")),
-        ('commands', (def_commands, "A short list of supported commands"))
+        ('help', (default_help_response, "This help text")),
+        ('commands', (default_commands_response, "A short list of supported commands"))
     ])
-    return defaults
+    return command_defaults
+
+def updated_default_commands(default_commands, bot_details):
+    if not bot_details['default_commands_enabled']:
+        return OrderedDict()
+    exclude_list = bot_details['commands'] or ('commands', 'help')
+    return OrderedDict((name,option) for name, option in default_commands.items()
+                       if name not in exclude_list)
 
-def sync_botdetails_defaultcommands(bot_details, default_commands):
     # Update default_commands from any changes in bot_details
-    if not bot_details['defaults']:  # Bot class will handle all commands
+    if not bot_details['default_commands_enabled']:  # Bot class will handle all commands
         default_commands = {}
     else:
         if len(bot_details['commands']) == 0:  # No commands specified, so don't use this feature
@@ -228,9 +244,6 @@ def sync_botdetails_defaultcommands(bot_details, default_commands):
             for command in bot_details['commands']:  # Bot commands override defaults
                 if command in default_commands:
                     del default_commands[command]
-    # Sync default_commands changes with bot_details
-    if len(default_commands) == 0:
-        bot_details['defaults'] = False
 
 def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name):
     # type: (Any, bool, str) -> Any
@@ -256,13 +269,16 @@ def run_message_handler_for_bot(lib_module, quiet, config_file, bot_name):
         'name': bot_name.capitalize(),
         'description': "",
         'commands': {},
-        'defaults': True,
+        'default_commands_enabled': True,
     }
     bot_details.update(getattr(lib_module.handler_class, 'META', {}))
 
     # Initialise default commands, then override & sync with bot_details
     default_commands = setup_default_commands(bot_details, message_handler)
-    sync_botdetails_defaultcommands(bot_details, default_commands)
+    updated_defaults = updated_default_commands(default_commands, bot_details)
+    # Update bot_details if updated_defaults is empty
+    if len(updated_defaults) == 0:
+        bot_details['default_commands_enabled'] = False
 
     if not quiet:
         print("Running {} Bot:".format(bot_details['name']))
@@ -288,16 +304,14 @@ def handle_message(message):
                 return
 
         if is_private_message or is_mentioned:
-            # Handle any default_commands first
-            if len(default_commands) > 0:
-                if '' in default_commands and len(message['content']) == 0:
-                    restricted_client.send_reply(message, default_commands[''][0]())
-                    return
-                for command in default_commands:
-                    if command == '':
-                        continue
-                    if message['content'].startswith(command):
-                        restricted_client.send_reply(message, default_commands[command][0]())
+            # Handle any default commands first
+            if len(updated_defaults) > 0:
+                for command in updated_defaults:
+                    if command == '' and len(message['content']) == 0:
+                        restricted_client.send_reply(message, updated_defaults[''][0]())
+                        return
+                    if command == message['content']:
+                        restricted_client.send_reply(message, updated_defaults[command][0]())
                         return
             # ...then pass anything else to bot to deal with
             message_handler.handle_message(