From 01820d2bc9bcc85e56daf2289b2124c8d8bd7862 Mon Sep 17 00:00:00 2001
From: Andrei <b1naryth1ef@gmail.com>
Date: Tue, 25 Apr 2017 14:23:47 -0700
Subject: [PATCH] Fix some issues with the way command regexes work

- Remove the limitation that prevented us from surpassing 100 commands
- Fix CommandEvent.name being the full command string, and not just the
command name
- Fix the error thrown on mismatched args being utter trash
---
 disco/bot/bot.py     |  2 +-
 disco/bot/command.py | 22 +++++++++++++---------
 tests/test_bot.py    | 23 ++++++++++++++++++++++-
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/disco/bot/bot.py b/disco/bot/bot.py
index 52b4aab..18260b9 100644
--- a/disco/bot/bot.py
+++ b/disco/bot/bot.py
@@ -237,7 +237,7 @@ class Bot(LoggingClass):
         Computes a single regex which matches all possible command combinations.
         """
         commands = list(self.commands)
-        re_str = '|'.join(command.regex for command in commands)
+        re_str = '|'.join(command.regex(grouped=False) for command in commands)
         if re_str:
             self.command_matches_re = re.compile(re_str, re.I)
         else:
diff --git a/disco/bot/command.py b/disco/bot/command.py
index c5aaddf..9a03ebe 100644
--- a/disco/bot/command.py
+++ b/disco/bot/command.py
@@ -6,6 +6,7 @@ from disco.bot.parser import ArgumentSet, ArgumentError
 from disco.util.functional import cached_property
 
 ARGS_REGEX = '(?: ((?:\n|.)*)$|$)'
+ARGS_UNGROUPED_REGEX = '(?: (?:\n|.)*$|$)'
 
 USER_MENTION_RE = re.compile('<@!?([0-9]+)>')
 ROLE_MENTION_RE = re.compile('<@&([0-9]+)>')
@@ -44,11 +45,11 @@ class CommandEvent(object):
         self.command = command
         self.msg = msg
         self.match = match
-        self.name = self.match.group(0)
+        self.name = self.match.group(1).strip()
         self.args = []
 
-        if self.match.group(1):
-            self.args = [i for i in self.match.group(1).strip().split(' ') if i]
+        if self.match.group(2):
+            self.args = [i for i in self.match.group(2).strip().split(' ') if i]
 
     @property
     def codeblock(self):
@@ -222,10 +223,9 @@ class Command(object):
         """
         A compiled version of this command's regex.
         """
-        return re.compile(self.regex, re.I)
+        return re.compile(self.regex(), re.I)
 
-    @property
-    def regex(self):
+    def regex(self, grouped=True):
         """
         The regex string that defines/triggers this command.
         """
@@ -235,10 +235,13 @@ class Command(object):
             group = ''
             if self.group:
                 if self.group in self.plugin.bot.group_abbrev:
-                    group = '{}(?:\w+)? '.format(self.plugin.bot.group_abbrev.get(self.group))
+                    group = '(?:\w+)? '.format(self.plugin.bot.group_abbrev.get(self.group))
                 else:
                     group = self.group + ' '
-            return '^{}(?:{})'.format(group, '|'.join(self.triggers)) + ARGS_REGEX
+            return ('^{}({})' if grouped else '^{}(?:{})').format(
+                group,
+                '|'.join(self.triggers)
+            ) + (ARGS_REGEX if grouped else ARGS_UNGROUPED_REGEX)
 
     def execute(self, event):
         """
@@ -251,9 +254,10 @@ class Command(object):
             Whether this command was successful
         """
         if len(event.args) < self.args.required_length:
-            raise CommandError('{} requires {} arguments (passed {})'.format(
+            raise CommandError(u'Command {} requires {} arguments (`{}`) passed {}'.format(
                 event.name,
                 self.args.required_length,
+                self.raw_args,
                 len(event.args)
             ))
 
diff --git a/tests/test_bot.py b/tests/test_bot.py
index 5650dc2..566de0a 100644
--- a/tests/test_bot.py
+++ b/tests/test_bot.py
@@ -2,6 +2,13 @@ from unittest import TestCase
 
 from disco.client import ClientConfig, Client
 from disco.bot.bot import Bot
+from disco.bot.command import Command
+
+
+class MockBot(Bot):
+    @property
+    def commands(self):
+        return getattr(self, '_commands', [])
 
 
 class TestBot(TestCase):
@@ -9,7 +16,7 @@ class TestBot(TestCase):
         self.client = Client(ClientConfig(
             {'config': 'TEST_TOKEN'}
         ))
-        self.bot = Bot(self.client)
+        self.bot = MockBot(self.client)
 
     def test_command_abbreviation(self):
         groups = ['config', 'copy', 'copez', 'copypasta']
@@ -24,3 +31,17 @@ class TestBot(TestCase):
         groups = ['cat', 'cap', 'caz', 'cas']
         result = self.bot.compute_group_abbrev(groups)
         self.assertDictEqual(result, {})
+
+    def test_many_commands(self):
+        self.bot._commands = [
+            Command(None, None, 'test{}'.format(i), '<test:str>')
+            for i in range(1000)
+        ]
+
+        self.bot.compute_command_matches_re()
+        match = self.bot.command_matches_re.match('test5 123')
+        self.assertNotEqual(match, None)
+
+        match = self.bot._commands[0].compiled_regex.match('test0 123 456')
+        self.assertEqual(match.group(1).strip(), 'test0')
+        self.assertEqual(match.group(2).strip(), '123 456')