Merge lp:~keegan-csmith/ibid/named-groups-656201 into lp:~ibid-core/ibid/old-trunk-1.6

[Keegan Carruthers-Smith]

Added named arguments to the match decorator.

Named arguments for the match decorator.

[Stefano Rivera (stefanor)]

Some plugins that could benefit from this: unichr (conversions), portfor (network), forget (memo), lookup_mac (sysadmin)

[Keegan Carruthers-Smith (keegan-csmith)]

I thought this would support repeating named arguments, but it does not. So gonna mark it as need fixing until I work out a way around it.

review: Needs Fixing

[lp:~keegan-csmith/ibid/named-groups-656201]

1017. By Keegan Carruthers-Smith on 2011-02-25

Allow a named argument to be used more than once for simple matches.

1018. By Keegan Carruthers-Smith on 2011-02-25

Changed name regex from \S+ to \w+.

[Max Rabkin (max-rabkin)]

@match(r'^I\s+am\s+(?P<nick>\S+)\s+on\s+(?P<network>\S+)$', simple=False)

This doesn't actually work: the name needs to be nick__1_, etc. Perhaps we should allow unmangled names as well: instead of

name = re.match(r'^(\S+)__\d+_$', name).group(1)

use

name = re.match(r'^(\S+)(?:__\d+_)?$', name).group(1)

review: Needs Fixing

[lp:~keegan-csmith/ibid/named-groups-656201]

1019. By Keegan Carruthers-Smith on 2011-02-27

Allow unmangled grouped names when using normal regexs.

[Keegan Carruthers-Smith (keegan-csmith)]

> Some plugins that could benefit from this: unichr (conversions), portfor
> (network), forget (memo), lookup_mac (sysadmin)
Just did portfor and forget. unichr looked too scary and lookup_mac wouldn't really benefit from it.

[Keegan Carruthers-Smith (keegan-csmith)]

> @match(r'^I\s+am\s+(?P<nick>\S+)\s+on\s+(?P<network>\S+)$', simple=False)
>
> This doesn't actually work: the name needs to be nick__1_, etc. Perhaps we
> should allow unmangled names as well: instead of
>
> name = re.match(r'^(\S+)__\d+_$', name).group(1)
>
> use
>
> name = re.match(r'^(\S+)(?:__\d+_)?$', name).group(1)

Done, but needs to be \S+?

Ok I think this is ready for reviews again.

review: Needs Resubmitting

[Keegan Carruthers-Smith (keegan-csmith)]

review: Abstain

[lp:~keegan-csmith/ibid/named-groups-656201]

1020. By Keegan Carruthers-Smith on 2011-02-27

Added my name to the Copyright header.

[Max Rabkin (max-rabkin)]

review: Approve

[Stefano Rivera (stefanor)]

I'd approve, but I have one issue. It's ambiguous in the documentation whether {url} grabs or not.

[Stefano Rivera (stefanor)]

Event: {'account': 1, 'responses': [], 'source': u'atrum', 'addressed': u'tibid', 'processed': False, 'time': datetime.datetime(2011, 2, 27, 16, 19, 43, 784287), 'identity': 1, 'message': {'raw': u'tibid: foo ~= s/[0-9]/X/r', 'deaddressed': u'foo ~= s/[0-9]/X/r', 'clean': u'foo ~= s/[0-9]/X/r', 'stripped': u'tibid: foo ~= s/[0-9]/X/r'}, 'type': u'message', 'public': True, 'channel': u'#ibid', 'sender': {'nick': u'tumbleweed', 'connection': u'tumbleweed!<email address hidden>', 'id': u'tumbleweed'}}
Traceback (most recent call last):
  File "/home/stefanor/bzr/ibid/stefanor/tibid/ibid/core.py", line 33, in _process
    processor.process(event)
  File "/home/stefanor/bzr/ibid/stefanor/tibid/ibid/plugins/__init__.py", line 148, in process
    "Can't intermix named and positional arguments.")
AssertionError: Can't intermix named and positional arguments.

review: Needs Fixing

[lp:~keegan-csmith/ibid/named-groups-656201]

1021. By Keegan Carruthers-Smith on 2011-02-28

Make factoid.modify only use named parameters.

1022. By Keegan Carruthers-Smith on 2011-02-28

Don't use group matches in selector docs.

[Keegan Carruthers-Smith (keegan-csmith)]

> I'd approve, but I have one issue. It's ambiguous in the documentation whether {url} grabs or not.
Reverted to old table in r1022

> AssertionError: Can't intermix named and positional arguments.
Fixed in r1021. This seems to be the only match which mix's named and positional arguments.

[Stefano Rivera (stefanor)]

separator != seperator

[lp:~keegan-csmith/ibid/named-groups-656201]

1023. By Keegan Carruthers-Smith on 2011-03-23

seperator != separator

[Stefano Rivera (stefanor)]

review: Approve

=== modified file 'AUTHORS'
--- AUTHORS	2011-03-11 21:59:06 +0000
+++ AUTHORS	2011-03-23 15:17:28 +0000
@@ -24,3 +24,4 @@
  * Kevin Woodland
  * Guy Halse
  * Dominic Cleal
+ * Keegan Carruthers-Smith
\ No newline at end of file

=== modified file 'docs/api/ibid.plugins.rst'
--- docs/api/ibid.plugins.rst	2010-06-13 22:23:43 +0000
+++ docs/api/ibid.plugins.rst	2011-03-23 15:17:28 +0000
@@ -147,7 +147,7 @@
    Selector     expands to
    ============ ==========================================
    ``{alpha}``  ``[a-zA-Z]+``
-   ``{any}``    ``.*``
+   ``{any}``    ``.+``
    ``{chunk}``  ``\S+``
    ``{digits}`` ``\d+``
    ``{number}`` ``\d*\.?\d+``
@@ -169,6 +169,21 @@
 
       @match(r'^(?:foo|bar)\s+(\S+)$', simple=False)
 
+   Optionally you can specify keyword arguments, which will get passed on to
+   the function. The syntax is ``{keyword:selector}``, where the keywords will
+   get passed onto the decorated method::
+
+      @match(r'I am {nick:chunk} on {network:chunk}')
+      def register(self, event, network, nick):
+         pass
+
+   The above match is equivalent to this non-simple version::
+
+      @match(r'^I\s+am\s+(?P<nick>\S+)\s+on\s+(?P<network>\S+)$', simple=False)
+
+   Note that you cannot mix positional and keyword arguments. All your
+   selectors must either be named, or all be unnamed.
+
    *version* can be set to one of:
 
    ``'clean'``

=== modified file 'ibid/plugins/__init__.py'
--- ibid/plugins/__init__.py	2011-01-21 20:30:46 +0000
+++ ibid/plugins/__init__.py	2011-03-23 15:17:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (c) 2008-2010, Michael Gorven, Stefano Rivera
+# Copyright (c) 2008-2011, Michael Gorven, Stefano Rivera, Keegan Carruthers-Smith
 # Released under terms of the MIT/X/Expat Licence. See COPYING for details.
 
 from copy import copy
@@ -23,7 +23,7 @@
             if not os.path.exists(os.path.join(x, *package + ['__init__.py']))]
 
 import ibid
-from ibid.compat import json
+from ibid.compat import json, defaultdict
 from ibid.utils import url_regex
 
 __path__ = pluginPackagePaths(__name__) + __path__
@@ -142,10 +142,27 @@
                         event.message[method.message_version])
                 if match is not None:
                     args = match.groups()
+                    kwargs = match.groupdict()
+                    if kwargs:
+                        assert len(args) == len(kwargs), (
+                            "Can't intermix named and positional arguments.")
+                        # Convert the names from the %s__%d_ format to %s
+                        args = {}
+                        for name, value in kwargs.iteritems():
+                            name = re.match(r'^(\S+?)(?:__\d+_)?$', name).group(1)
+                            if args.get(name, None) is None:
+                                args[name] = value
+                            else:
+                                assert value is None, (
+                                    'named argument %s was matched more '
+                                    'than once.' % name)
             if args is not None:
                 if (not getattr(method, 'auth_required', False)
                         or auth_responses(event, self.permission)):
-                    method(event, *args)
+                    if isinstance(args, dict):
+                        method(event, **args)
+                    else:
+                        method(event, *args)
                 elif not getattr(method, 'auth_fallthrough', True):
                     event.processed = True
 
@@ -214,7 +231,7 @@
 def _match_sub_selectors(regex):
     selector_patterns = {
         'alpha'   : r'[a-zA-Z]+',
-        'any'     : r'.*',
+        'any'     : r'.+',
         'chunk'   : r'\S+',
         'digits'  : r'\d+',
         'number'  : r'\d*\.?\d+',
@@ -224,12 +241,22 @@
 
     regex = regex.replace(' ', r'(?:\s+)')
 
-    for pattern in re.finditer('{(%s)}' % '|'.join(selector_patterns.keys()),
-                               regex):
-        pattern = pattern.group(1)
-        old = '{%s}' % pattern
-        new = '(%s)' % selector_patterns[pattern]
-        regex = regex.replace(old, new)
+    name_count = defaultdict(int)
+    def selector_to_re(match):
+        name    = match.group(1)
+        pattern = match.group(2)
+
+        if name is None:
+            return '(%s)' % selector_patterns[pattern]
+
+        # Prevent conflicts when reusing a name
+        name_count[name] += 1
+        name = '%s__%d_' % (name, name_count[name])
+
+        return '(?P<%s>%s)' % (name, selector_patterns[pattern])
+
+    regex = re.sub(r'{(?:(\w+):)?(%s)}' % '|'.join(selector_patterns.keys()),
+                   selector_to_re, regex)
 
     if not regex.startswith('^'):
         regex = '^' + regex

=== modified file 'ibid/plugins/factoid.py'
--- ibid/plugins/factoid.py	2011-02-24 19:07:10 +0000
+++ ibid/plugins/factoid.py	2011-03-23 15:17:28 +0000
@@ -684,9 +684,12 @@
                     event.identity, event.sender['connection'])
             event.addresponse(True)
 
-    @match(r'^(.+?)(?:\s+#(\d+)|\s+/(.+?)/(r?))?\s*(?:~=|=~)\s*([sy](?P<sep>.).+(?P=sep).*(?P=sep)[gir]*)$')
+    @match(r'(?P<name>.+?)'
+           r'(?: #{number:digits}| /(?P<pattern>.+?)/(?P<is_regex>r?))?'
+           r'\s*(?:~=|=~)\s*'
+           r'(?P<operation>[sy](?P<sep>.).+(?P=sep).*(?P=sep)[gir]*)$')
     @authorise(fallthrough=False)
-    def modify(self, event, name, number, pattern, is_regex, operation, separator):
+    def modify(self, event, name, number, pattern, is_regex, operation, sep):
         factoids = get_factoid(event.session, name, number, pattern, is_regex, all=True)
         if len(factoids) == 0:
             if pattern:
@@ -707,6 +710,7 @@
                 return
 
             # Not very pythonistic, but escaping is a nightmare.
+            separator = sep
             parts = [[]]
             pos = 0
             while pos < len(operation):

=== modified file 'ibid/plugins/memo.py'
--- ibid/plugins/memo.py	2011-03-11 21:59:06 +0000
+++ ibid/plugins/memo.py	2011-03-23 15:17:28 +0000
@@ -151,18 +151,18 @@
             'source': to.source,
         })
 
-    @match(r'^(?:delete|forget)\s+(?:my\s+)?'
-            r'(?:(first|last|\d+(?:st|nd|rd|th)?)\s+)?' # 1st way to specify number
-            r'(?:memo|message|msg)\s+'
-            r'(?(1)|#?(\d+)\s+)?' # 2nd way
-            r'(?:for|to)\s+(.+?)(?:\s+on\s+(\S+))?$')
+    @match(r'(?:delete|forget) (?:my )?'
+            r'(?:(?P<num>first|last|\d+(?:st|nd|rd|th)?) )?' # 1st way to specify number
+            r'(?:memo|message|msg) '
+            r'(?(1)|#?{num:digits} )?' # 2nd way
+            r'(?:for|to) (?P<who>.+?)(?: on {source:chunk})?')
     @authorise(fallthrough=False)
-    def forget(self, event, num1, num2, who, source):
+    def forget(self, event, num, who, source):
         if not source:
             source = event.source
         else:
             source = source.lower()
-        number = num1 or num2 or 'last'
+        number = num or 'last'
         number = number.lower()
         if number == 0:
             # Don't wrap around to last message, that'd be unexpected

=== modified file 'ibid/plugins/network.py'
--- ibid/plugins/network.py	2011-02-20 18:44:54 +0000
+++ ibid/plugins/network.py	2011-03-23 15:17:28 +0000
@@ -528,10 +528,10 @@
                             break
                         self.protocols[proto.lower()].append(port)
 
-    @match(r'^(?:(.+)\s+)?ports?(?:\s+numbers?)?(?(1)|\s+for\s+(.+))$')
-    def portfor(self, event, proto1, proto2):
+    @match(r'(?:{proto:any} )?ports?(?: numbers?)?(?(1)| for {proto:any})')
+    def portfor(self, event, proto):
         self._load_services()
-        protocol = (proto1 or proto2).lower()
+        protocol = proto.lower()
         if protocol in self.protocols:
             event.addresponse(human_join(self.protocols[protocol]))
         else:

=== modified file 'ibid/plugins/social.py'
--- ibid/plugins/social.py	2011-02-21 16:19:25 +0000
+++ ibid/plugins/social.py	2011-03-23 15:17:28 +0000
@@ -27,7 +27,7 @@
 
     features = ('lastfm',)
 
-    @match(r'^last\.?fm\s+for\s+(\S+?)\s*$')
+    @match(r'last\.?fm for {username:chunk}')
     def listsongs(self, event, username):
         songs = feedparser.parse('http://ws.audioscrobbler.com/1.0/user/%s/recenttracks.rss?%s' % (username, time()))
         if songs['bozo']:
@@ -147,7 +147,7 @@
     def twitter(self, event, id):
         self.update(event, u'twitter', id)
 
-    @match(r'^https?://(?:www\.)?identi.ca/notice/(\d+)$')
+    @match(r'https?://(?:www\.)?identi.ca/notice/{id:digits}')
     def identica(self, event, id):
         self.update(event, u'identica', id)