Better error messages for invalid actions.

3 files changed, 27 insertions, 6 deletions

diff --git a/NEWS.txt b/NEWS.txt
index 8e8c83b..4e527c4 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -1,3 +1,11 @@
+What's New in argparse 1.1
+==========================
+
+- Issue #36: Get "from argparse import *" working
+- Issue #37: Stop treating two character alphanumeric flags as numbers
+- Issue #39: Better error messages for invalid actions
+
+
 What's New in argparse 1.0.1
 ============================
 
diff --git a/argparse.py b/argparse.py
index bfa966c..503cbaf 100644
--- a/argparse.py
+++ b/argparse.py
@@ -118,6 +118,10 @@ except NameError:
             result.reverse()
         return result
 
+
+def _callable(obj):
+    return hasattr(obj, '__call__') or hasattr(obj, '__bases__')
+
 # silence Python 2.6 buggy warnings about Exception.message
 if _sys.version_info[:2] == (2, 6):
     import warnings
@@ -1261,6 +1265,8 @@ class _ActionsContainer(object):
 
         # create the action object, and add it to the parser
         action_class = self._pop_action_class(kwargs)
+        if not _callable(action_class):
+            raise ValueError('unknown action "%s"' % action_class)
         action = action_class(**kwargs)
         return self._add_action(action)
 
@@ -2166,10 +2172,9 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer):
 
     def _get_value(self, action, arg_string):
         type_func = self._registry_get('type', action.type, action.type)
-        if not hasattr(type_func, '__call__'):
-            if not hasattr(type_func, '__bases__'): # classic classes
-                msg = _('%r is not callable')
-                raise ArgumentError(action, msg % type_func)
+        if not _callable(type_func):
+            msg = _('%r is not callable')
+            raise ArgumentError(action, msg % type_func)
 
         # convert the value to the appropriate type
         try:
diff --git a/test/test_argparse.py b/test/test_argparse.py
index 0bd06ca..c1717dc 100644
--- a/test/test_argparse.py
+++ b/test/test_argparse.py
@@ -3613,8 +3613,16 @@ class TestInvalidArgumentConstructors(TestCase):
         self.assertValueError('---')
 
     def test_invalid_action(self):
-        self.assertTypeError('-x', action='foo')
-        self.assertTypeError('foo', action='baz')
+        self.assertValueError('-x', action='foo')
+        self.assertValueError('foo', action='baz')
+        parser = argparse.ArgumentParser()
+        try:
+            parser.add_argument("--foo", action="store-true")
+        except ValueError:
+            e = sys.exc_info()[1]
+            expected = 'unknown action'
+            msg = 'expected %r, found %r' % (expected, e)
+            self.failUnless(expected in str(e), msg)
 
     def test_no_argument_actions(self):
         for action in ['store_const', 'store_true', 'store_false',