New tests, and new implementation (though a test still fails).

src/commands.py

@@ -1,5 +1,3 @@
-#!/usr/bin/env python
-
 ###
 # Copyright (c) 2002-2004, Jeremiah Fincher
 # All rights reserved.
@@ -59,6 +57,9 @@ import supybot.structures as structures
 # Non-arg wrappers -- these just change the behavior of a command without
 # changing the arguments given to it.
 ###
+
+# Thread has to be a non-arg wrapper because by the time we're parsing and
+# validating arguments, we're inside the function we'd want to thread.
 def thread(f):
     """Makes sure a command spawns a thread when called."""
     def newf(self, irc, msg, args, *L, **kwargs):
@@ -141,11 +142,8 @@ decorators = ircutils.IrcDict({
 
 
 ###
-# Arg wrappers, wrappers that add arguments to the command.  They accept the
+# Converters, which take irc, msg, args, and a state object, and build up the
-# irc, msg, and args, of course, as well as a State object which holds the args
+# validated and converted args for the method in state.args.
-# (and kwargs, though none currently take advantage of that) to be given to the
-# command being decorated, as well as the name of the command, the plugin, the
-# log, etc.
 ###
 
 # This is just so we can centralize this, since it may change.
@@ -396,8 +394,8 @@ def public(irc, msg, args, state, errmsg=None):
 def checkCapability(irc, msg, args, state, cap):
     cap = ircdb.canonicalCapability(cap)
     if not ircdb.checkCapability(msg.prefix, cap):
-        state.log.warning('%s tried %s without %s.',
+##         state.log.warning('%s tried %s without %s.',
-                          msg.prefix, state.name, cap)
+##                           msg.prefix, state.name, cap)
         irc.errorNoCapability(cap, Raise=True)
 
 def anything(irc, msg, args, state):
@@ -507,16 +505,15 @@ def getConverter(name):
 def callConverter(name, irc, msg, args, state, *L):
     getConverter(name)(irc, msg, args, state, *L)
 
-class State(object):
+###
-    def __init__(self, name=None, logger=None):
+# Contexts.  These determine what the nature of conversions is; whether they're
-        if logger is None:
+# defaulted, or many of them are allowed, etc.  Contexts should be reusable;
-            logger = log
+# i.e., they should not maintain state between calls.
-        self.args = []
+###
-        self.kwargs = {}
+def contextify(spec):
-        self.name = name
+    if not isinstance(spec, context):
-        self.log = logger
+        spec = context(spec)
-        self.getopts = []
+    return spec
-        self.channel = None
 
 class context(object):
     def __init__(self, spec):
@@ -551,9 +548,9 @@ class additional(context):
 class optional(additional):
     def __call__(self, irc, msg, args, state):
         try:
-            self.__parent.__call__(irc, msg, args, state)
+            super(optional, self).__call__(irc, msg, args, state)
         except (callbacks.ArgumentError, callbacks.Error), e:
-            log.debug('Got %s, returning default.', utils.exntoString(e))
+            log.debug('Got %s, returning default.', utils.exnToString(e))
             state.args.append(self.default)
 
 class any(context):
@@ -571,153 +568,98 @@ class any(context):
 
 class many(any):
     def __call__(self, irc, msg, args, state):
+        context.__call__(self, irc, msg, args, state)
         super(many, self).__call__(irc, msg, args, state)
-        assert state.args
-        if not state.args[-1]:
-            raise callbacks.ArgumentError
 
-# getopts:  None means "no conversion", '' means "takes no argument"
+class getopts(context):
-def args(irc,msg,args, types=[], state=None,
+    """The empty string indicates that no argument is taken; None indicates
-         getopts=None, allowExtra=False, requireExtra=False, combineRest=True):
+    that there is no converter for the argument."""
-    if state is None:
+    def __init__(self, getopts):
-        state = State(name='unknown', logger=log)
+        self.getopts = {}
-    if requireExtra:
+        self.getoptL = []
-        # Implied by requireExtra.
+        for (name, spec) in getopts.iteritems():
-        allowExtra = True
+            if spec == '':
-        combineRest = False
+                self.getoptL.append(name)
-    types = types[:] # We're going to destroy this.
+                self.getopts[name] = None
-    if getopts is not None:
+            else:
-        getoptL = []
+                self.getoptL.append(name + '=')
-        for (key, value) in getopts.iteritems():
+                self.getopts[name] = contextify(spec)
-            if value != '': # value can be None, remember.
-                key += '='
-            getoptL.append(key)
-        log.debug('getoptL: %r', getoptL)
 
-    def callWrapper(spec):
+    def __call__(self, irc, msg, args, state):
-        if isinstance(spec, tuple):
+        (optlist, args) = getopt.getopt(args, '', self.getoptL)
-            assert spec, 'tuple specification cannot be empty.'
+        getopts = []
-            name = spec[0]
-            specArgs = spec[1:]
-        else:
-            assert isinstance(spec, basestring) or spec is None
-            name = spec
-            specArgs = ()
-        if name is None:
-            name = 'anything'
-        enforce = True
-        optional = False
-        if name.startswith('?'):
-            optional = True
-            name = name[1:]
-        elif name.endswith('?'):
-            optional = True
-            enforce = False
-            name = name[:-1]
-        elif name[-1] in '*+':
-            name = name[:-1]
-        default = ''
-        wrapper = wrappers[name]
-        if optional and specArgs:
-            # First arg is default.
-            default = specArgs[0]
-            specArgs = specArgs[1:]
-            if callable(default):
-                default = default()
-        try:
-            wrapper(irc, msg, args, state, *specArgs)
-        except (callbacks.Error, ValueError, callbacks.ArgumentError), e:
-            state.log.debug('%r when calling wrapper.', utils.exnToString(e))
-            if not enforce:
-                state.args.append(default)
-            else:
-                state.log.debug('Re-raising %s because of enforce.', e)
-                raise
-        except IndexError, e:
-            state.log.debug('%r when calling wrapper.', utils.exnToString(e))
-            if optional:
-                state.args.append(default)
-            else:
-                state.log.debug('Raising ArgumentError because of '
-                                'non-optional args: %r', spec)
-                raise callbacks.ArgumentError
-
-    # First, we getopt stuff.
-    if getopts is not None:
-        (optlist, args) = getopt.getopt(args, '', getoptL)
         for (opt, arg) in optlist:
             opt = opt[2:] # Strip --
-            log.debug('getopt %s: %r', opt, arg)
+            context = self.getopts[opt]
-            if getopts[opt] != '':
+            if context is not None:
-                # This is a MESS.  But I can't think of a better way to do it.
+                st = state.essence()
-                originalArgs = args
+                context(irc, msg, args, st)
-                args = [arg]
+                assert len(st.args) == 1
-                originalStateArgsLen = len(state.args)
+                getopts.append((opt, st.args[0]))
-                callWrapper(getopts[opt])
-                args = originalArgs
-                if originalStateArgsLen < len(state.args):
-                    assert originalStateArgsLen == len(state.args)-1
-                    arg = state.args.pop()
             else:
-                    arg = None
+                getopts.append((opt, True))
-                state.getopts.append((opt, arg))
+        state.args.append(getopts)
-            else:
-                state.getopts.append((opt, True))
-    #log.debug('Finished getopts: %s', state.getopts)
                 
-    # Second, we get out everything but the last argument (or, if combineRest
+
-    # is False, we'll clear out all the types).
+
-    while len(types) > 1 or (types and not combineRest):
+###
-        callWrapper(types.pop(0))
+# This is our state object, passed to converters along with irc, msg, and args.
-    # Third, if there is a remaining required or optional argument
+###
-    # (there's a possibility that there were no required or optional
+class State(object):
-    # arguments) then we join the remaining args and work convert that.
+    log = log
-    if types:
+    def __init__(self):
-        name = types[0]
+        self.args = []
-        if isinstance(name, tuple):
+        self.kwargs = {}
-            name = name[0]
+        self.channel = None
-        if name[-1] in '*+':
+
-            originalStateArgs = state.args
+    def essence(self):
-            state.args = []
+        st = State()
-            if name.endswith('+'):
+        for (attr, value) in self.__dict__.iteritems():
-                callWrapper(types[0]) # So it raises an error if no args.
+            if attr not in ('args', 'kwargs', 'channel'):
-            while args:
+                setattr(st, attr, value)
-                callWrapper(types[0])
+        return st
-            lastArgs = state.args
+
-            state.args = originalStateArgs
+###
-            state.args.append(lastArgs)
+# This is a compiled Spec object.
-        else:
+###
-            if args:
+class Spec(object):
-                rest = ' '.join(args)
+    def _state(self, attrs={}):
-                args = [rest]
+        st = State()
-            callWrapper(types.pop(0))
+        st.__dict__.update(attrs)
-    if args and not allowExtra and isinstance(args, list):
+        return st
-        # args could be a regexp in a urlSnarfer.
+    
-        log.debug('args but not allowExtra: %r', args)
+    def __init__(self, types, allowExtra=False, combineRest=True):
+        self.types = types
+        self.allowExtra = allowExtra
+        self.combineRest = combineRest
+        utils.mapinto(contextify, self.types)
+
+    def __call__(self, irc, msg, args, stateAttrs={}):
+        state = self._state(stateAttrs)
+        if self.types:
+            types = self.types[:]
+            while types:
+                if len(types) == 1 and self.combineRest and args:
+                    break
+                context = types.pop(0)
+                context(irc, msg, args, state)
+            if types and args:
+                assert self.combineRest
+                args[:] = [' '.join(args)]
+                types[0](irc, msg, args, state)
+        if args and not self.allowExtra:
             raise callbacks.ArgumentError
-    if requireExtra and not args:
-        log.debug('requireExtra and not args: %r', args)
-    log.debug('command.args args: %r' % args)
-    log.debug('command.args state.args: %r' % state.args)
-    log.debug('command.args state.getopts: %r' % state.getopts)
         return state
 
-# These are used below, but we need to rename them so their names aren't
+# This is used below, but we need to rename it so its name isn't
 # shadowed by our locals.
-_args = args
 _decorators = decorators
-def wrap(f, *argsArgs, **argsKwargs):
+def wrap(f, specList, decorators=None, **kw):
+    spec = Spec(specList, **kw)
     def newf(self, irc, msg, args, **kwargs):
-        state = State('%s.%s' % (self.name(), f.func_name), self.log)
+        spec(irc, msg, args, stateAttrs={'cb': self, 'log': self.log})
-        state.cb = self # This should probably be in State.__init__.
-        _args(irc,msg,args, state=state, *argsArgs, **argsKwargs)
-        if 'getopts' in argsKwargs:
-            f(self, irc, msg, args, state.getopts, *state.args, **state.kwargs)
-        else:
         f(self, irc, msg, args, *state.args, **state.kwargs)
-
     newf = utils.changeFunctionName(newf, f.func_name, f.__doc__)
-    decorators = argsKwargs.pop('decorators', None)
     if decorators is not None:
         decorators = map(_decorators.__getitem__, decorators)
         for decorator in decorators:
@@ -725,6 +667,9 @@ def wrap(f, *argsArgs, **argsKwargs):
     return newf
 
 
-__all__ = ['wrap', 'args',
+__all__ = ['wrap', 'context', 'additional', 'optional', 'any',
-           'getConverter', 'addConverter', 'callConverter']
+           'many', 'getopts', 'getConverter', 'addConverter', 'callConverter']
+
+if world.testing:
+    __all__.append('Spec')
 # vim:set shiftwidth=4 tabstop=8 expandtab textwidth=78:

test/test_commands.py

@@ -0,0 +1,83 @@
+###
+# Copyright (c) 2002-2004, Jeremiah Fincher
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+#   * Redistributions of source code must retain the above copyright notice,
+#     this list of conditions, and the following disclaimer.
+#   * Redistributions in binary form must reproduce the above copyright notice,
+#     this list of conditions, and the following disclaimer in the
+#     documentation and/or other materials provided with the distribution.
+#   * Neither the name of the author of this software nor the name of
+#     contributors to this software may be used to endorse or promote products
+#     derived from this software without specific prior written consent.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+###
+
+from testsupport import *
+
+from supybot.commands import *
+import supybot.ircmsgs as ircmsgs
+import supybot.callbacks as callbacks
+
+
+class CommandsTestCase(SupyTestCase):
+    msg = ircmsgs.privmsg('test', 'foo')
+    class irc:
+        nick = 'test'
+        def error(self, s):
+            raise self.failureException, s
+    def assertState(self, spec, given, expected, **kwargs):
+        irc = callbacks.SimpleProxy(self.irc(), self.msg)
+        myspec = Spec(spec, **kwargs)
+        state = myspec(irc, self.msg, given)
+        self.assertEqual(state.args, expected,
+                         'Expected %r, got %r' % (expected, state.args))
+        
+    def testSpecInt(self):
+        self.assertState(['int'], ['1'], [1])
+        self.assertState(['int', 'int', 'int'], ['1', '2', '3'], [1, 2, 3])
+
+    def testRestHandling(self):
+        self.assertState([None], ['foo', 'bar', 'baz'], ['foo bar baz'])
+
+    def testOptional(self):
+        spec = [optional('int', 999), None]
+        self.assertState(spec, ['12', 'foo'], [12, 'foo'])
+        self.assertState(spec, ['foo'], [999, 'foo'])
+
+    def testAdditional(self):
+        spec = [additional('int', 999)]
+        self.assertState(spec, ['12'], [12])
+        self.assertState(spec, [], [999])
+        self.assertRaises(callbacks.Error,
+                          self.assertState, spec, ['foo'], ['asdf'])
+
+    def testGetopts(self):
+        spec = ['int', getopts({'foo': None, 'bar': 'int'}), 'int']
+        self.assertState(spec,
+                         ['12', '--foo', 'baz', '--bar', '13', '15'],
+                         [12, [('foo', 'baz'), ('bar', 13)], 15])
+        
+##     def testAny(self):
+##         self.assertState([None, any('int'), None],
+##                          ['foo', 'bar'],
+##                          ['foo', [], 'bar'])
+        
+
+
+# vim:set shiftwidth=4 tabstop=8 expandtab textwidth=78:
+