Bug 342948 – Add exception handling to GOption

svn path=/trunk/; revision=681

6 files changed, 107 insertions, 18 deletions

diff --git a/ChangeLog b/ChangeLog
index 1a0518be..00cd4808 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2007-04-30  Johannes Hölzl  <johannes.hoelzl@gmx.de>
+
+	* gobject/gobjectmodule.c (init_gobjectmodule),
+	* gobject/pygoptiongroup.c (arg_func),
+	* gobject/option.py (OptionParser._parse_args, OptionGroup._to_goptiongroup),
+	* tests/test_option.py: OptParse-Exceptions in GOption-callbacks
+	are now convertet into an GError.  GError from the
+	GOptionGroup.run is convertet into an OptParse-Exception.
+
 2007-07-06  Ed Catmur  <ed@catmur.co.uk>
 
 	* gobject/pygtype.c (pyg_param_gvalue_from_pyobject),
diff --git a/gobject/gobjectmodule.c b/gobject/gobjectmodule.c
index bf7b42b2..20003b9f 100644
--- a/gobject/gobjectmodule.c
+++ b/gobject/gobjectmodule.c
@@ -3028,7 +3028,7 @@ pyg_error_check(GError **error)
  * Returns: 0 if no exception has been raised, -1 if it is a
  * valid gobject.GError, -2 otherwise.
  */
-static gboolean
+gboolean
 pyg_gerror_exception_check(GError **error)
 {
     PyObject *type, *value, *traceback;
@@ -3634,6 +3634,8 @@ init_gobject(void)
   
     PyModule_AddStringConstant(m, "OPTION_REMAINING", G_OPTION_REMAINING);
 
+    PyModule_AddStringConstant(m, "OPTION_ERROR", (char*) g_quark_to_string(G_OPTION_ERROR));
+
     PyModule_AddIntConstant(m, "SPAWN_LEAVE_DESCRIPTORS_OPEN",
 			    G_SPAWN_LEAVE_DESCRIPTORS_OPEN);
     PyModule_AddIntConstant(m, "SPAWN_DO_NOT_REAP_CHILD",
diff --git a/gobject/option.py b/gobject/option.py
index ce869d15..14f7780a 100644
--- a/gobject/option.py
+++ b/gobject/option.py
@@ -31,11 +31,16 @@ GOptionGroup in gobject.
 
 import sys
 import optparse
-from optparse import OptionError
+from optparse import OptParseError, OptionError, OptionValueError, \
+                     BadOptionError, OptionConflictError
 import _gobject as gobject
 
 __all__ = [
+    "OptParseError",
     "OptionError",
+    "OptionValueError",
+    "BadOptionError",
+    "OptionConflictError"
     "Option",
     "OptionGroup",
     "OptionParser",
@@ -172,7 +177,15 @@ class OptionGroup(optparse.OptionGroup):
                 opt = self._long_opt[option_name]
             else:
                 opt = self._short_opt[option_name]
-            opt.process(option_name, option_value, self.values, parser)
+
+            try:
+                opt.process(option_name, option_value, self.values, parser)
+            except OptionValueError, error:
+            	gerror = gobject.GError(str(error))
+            	gerror.domain = gobject.OPTION_ERROR
+            	gerror.code = gobject.OPTION_ERROR_BAD_VALUE
+            	gerror.message = str(error)
+            	raise gerror
 
         group = gobject.OptionGroup(self.name, self.description,
                                     self.help_description, callback)
@@ -301,5 +314,19 @@ class OptionParser(optparse.OptionParser):
         context = self._to_goptioncontext(values)
         largs.extend(context.parse([sys.argv[0]] + rargs))
 
+    def parse_args(self, args=None, values=None):
+        try:
+            return optparse.OptionParser.parse_args(self, args, values)
+        except gobject.GError, error:
+            if error.domain != gobject.OPTION_ERROR:
+            	raise
+            if error.code == gobject.OPTION_ERROR_BAD_VALUE:
+                raise OptionValueError(error.message)
+            elif error.code == gobject.OPTION_ERROR_UNKNOWN_OPTION:
+    	        raise BadOptionError(error.message)
+            elif error.code == gobject.OPTION_ERROR_FAILED:
+                raise OptParseError(error.message)
+            else:
+                raise
 
 make_option = Option
diff --git a/gobject/pygobject-private.h b/gobject/pygobject-private.h
index 45e431cc..4b41338b 100644
--- a/gobject/pygobject-private.h
+++ b/gobject/pygobject-private.h
@@ -101,6 +101,8 @@ PyObject *pyg_integer_richcompare(PyObject *v,
                                   PyObject *w,
                                   int op);
 
+gboolean pyg_gerror_exception_check(GError **error);
+
 /* from pygtype.h */
 extern PyTypeObject PyGTypeWrapper_Type;
 
diff --git a/gobject/pygoptiongroup.c b/gobject/pygoptiongroup.c
index 09058d80..64654017 100644
--- a/gobject/pygoptiongroup.c
+++ b/gobject/pygoptiongroup.c
@@ -25,6 +25,7 @@
 #endif
 
 #include "pygobject-private.h"
+#include "pygobject.h"
 
 static gboolean
 check_if_owned(PyGOptionGroup *self)
@@ -38,8 +39,8 @@ check_if_owned(PyGOptionGroup *self)
     return FALSE;
 }
 
-
-static void destroy_g_group(PyGOptionGroup *self)
+static void
+destroy_g_group(PyGOptionGroup *self)
 {
     PyGILState_STATE state;
     state = pyg_gil_state_ensure();
@@ -100,30 +101,37 @@ static gboolean
 arg_func(const gchar *option_name,
          const gchar *value,
          PyGOptionGroup *self,
-         GError *error)
+         GError **error)
 {
     PyObject *ret;
     PyGILState_STATE state;
-
+    gboolean no_error;
+        
     state = pyg_gil_state_ensure();
   
     if (value == NULL)
+    {
         ret = PyObject_CallFunction(self->callback, "sOO", 
                                     option_name, Py_None, self);
+    }
     else
+    {
         ret = PyObject_CallFunction(self->callback, "ssO", 
                                     option_name, value, self);
+    }
     
-    if (ret == NULL)
-        PyErr_Print();
-
-    pyg_gil_state_release(state);
-
-    if (ret == NULL)
-        return FALSE;
-
-    Py_DECREF(ret);
-    return TRUE;      
+    if (ret != NULL)
+    {
+        Py_DECREF(ret);
+        pyg_gil_state_release(state);
+        return TRUE;
+    }
+    else
+    {
+        no_error = pyg_gerror_exception_check(error) != -1;
+        pyg_gil_state_release(state);
+        return no_error;
+    }
 }
 
 static PyObject *
diff --git a/tests/test_option.py b/tests/test_option.py
index 84e9305a..3996659e 100644
--- a/tests/test_option.py
+++ b/tests/test_option.py
@@ -1,13 +1,19 @@
 #!/usr/bin/env python
 
 import unittest
+import sys
 
+from StringIO import StringIO
 from common import gobject
 from gobject import option
 
 class TestOption(unittest.TestCase):
+    EXCEPTION_MESSAGE = "This callback fails"
 
     def setup_group(self):
+        def option_callback(option, opt, value, parser):
+            raise StandardError(self.EXCEPTION_MESSAGE)
+        
         group = option.OptionGroup(
             "unittest", "Unit test options", "Show all unittest options",
             option_list = [
@@ -15,6 +21,15 @@ class TestOption(unittest.TestCase):
                                    type="filename",
                                    dest="unit_file",
                                    help="Unit test option"),
+                option.make_option("--test-integer",
+                                   type="int",
+                                   dest="test_integer",
+                                   help="Unit integer option"),
+                option.make_option("--callback-failure-test",
+                                   action="callback",
+                                   callback=option_callback,
+                                   dest="test_integer",
+                                   help="Unit integer option"),
             ])
         group.add_option("-t", "--test", 
                          action="store_false", 
@@ -24,7 +39,8 @@ class TestOption(unittest.TestCase):
         return group
 
     def setup_parser(self):
-        parser = option.OptionParser("NAMES...", description="Option unit test")
+        parser = option.OptionParser("NAMES...", 
+                                     description="Option unit test")
         parser.add_option("-t", "--test", help="Unit test option",
                           action="store_false", dest="test", default=True)
         return parser
@@ -38,6 +54,31 @@ class TestOption(unittest.TestCase):
         assert group.values.test
         assert not parser.values.test
         assert group.values.unit_file == "test"
+        
+        try:
+            parser.parse_args(["test_option.py", "--test-integer=text"])
+        except option.OptionValueError:
+            pass
+        else:
+            assert False
+
+        sio = StringIO()
+        old_stderr = sys.stderr
+        sys.stderr = sio
+        try:
+            parser.parse_args(["test_option.py", "--callback-failure-test"])
+        finally:
+            sys.stderr = old_stderr
+        assert (sio.getvalue().split('\n')[-2] == 
+                "StandardError: " + self.EXCEPTION_MESSAGE)
+
+        try:
+            parser.parse_args(["test_option.py", "--unknwon-option"])
+        except option.BadOptionError:
+            pass
+        else:
+            assert False
 
     def testBadConstructor(self):
         self.assertRaises(TypeError, option.OptionGroup)
+