Improve error handling for enum/flags, rename the converter functions to

2007-06-30  Johan Dahlin  <jdahlin@async.com.br>

    * gtk/gtkbuilder.c:
    * gtk/gtkbuilder.h:
    * gtk/gtkbuilderparser.c:
    * gtk/gtkbuilderprivate.h:
    * gtk/gtkwidget.c: 
    * tests/buildertest.c:

    Improve error handling for enum/flags, rename the converter functions
    to be consistent. Add tests. Fixes #452465


svn path=/trunk/; revision=18312

5 files changed, 158 insertions, 101 deletions

diff --git a/gtk/gtkbuilder.c b/gtk/gtkbuilder.c
index 22228acdc1..167a815b9b 100644
--- a/gtk/gtkbuilder.c
+++ b/gtk/gtkbuilder.c
@@ -49,8 +49,10 @@ static void gtk_builder_get_property   (GObject         *object,
                                         GParamSpec      *pspec);
 static GType gtk_builder_real_get_type_from_name (GtkBuilder  *builder,
                                                   const gchar *type_name);
-static gint _gtk_builder_enum_from_string        (GType        type, 
-                                                  const gchar *string);
+static gboolean _gtk_builder_enum_from_string (GType         type, 
+					       const gchar  *string,
+					       gint         *enum_value,
+					       GError      **error);
 
 
 enum {
@@ -1038,7 +1040,7 @@ gtk_builder_value_from_string_type (GtkBuilder   *builder,
       {
         gboolean b;
 
-	if (!_gtk_builder_parse_boolean (string, &b, error))
+	if (!_gtk_builder_boolean_from_string (string, &b, error))
 	  {
 	    ret = FALSE;
 	    break;
@@ -1093,11 +1095,27 @@ gtk_builder_value_from_string_type (GtkBuilder   *builder,
         break;
       }
     case G_TYPE_ENUM:
-      g_value_set_enum (value, _gtk_builder_enum_from_string (type, string));
-      break;
+      {
+	gint enum_value;
+	if (!_gtk_builder_enum_from_string (type, string, &enum_value, error))
+	  {
+	    ret = FALSE;
+	    break;
+          }
+	g_value_set_enum (value, enum_value);
+	break;
+      }
     case G_TYPE_FLAGS:
-      g_value_set_flags (value, _gtk_builder_flags_from_string (type, string));
-      break;
+      {
+	gint flags_value;
+	if (!_gtk_builder_flags_from_string (type, string, &flags_value, error))
+	  {
+	    ret = FALSE;
+	    break;
+          }
+	g_value_set_flags (value, flags_value);
+	break;
+      }
     case G_TYPE_FLOAT:
     case G_TYPE_DOUBLE:
       {
@@ -1195,42 +1213,57 @@ gtk_builder_value_from_string_type (GtkBuilder   *builder,
   return ret;
 }
 
-static gint
-_gtk_builder_enum_from_string (GType        type, 
-                               const gchar *string)
+static gboolean
+_gtk_builder_enum_from_string (GType         type, 
+                               const gchar  *string,
+			       gint         *enum_value,
+			       GError      **error)
 {
   GEnumClass *eclass;
   GEnumValue *ev;
   gchar *endptr;
-  gint ret = 0;
-
+  gint value;
+  
   g_return_val_if_fail (G_TYPE_IS_ENUM (type), 0);
   g_return_val_if_fail (string != NULL, 0);
   
-  ret = strtoul (string, &endptr, 0);
+  value = strtoul (string, &endptr, 0);
   if (endptr != string) /* parsed a number */
-    return ret;
-
-  eclass = g_type_class_ref (type);
-  ev = g_enum_get_value_by_name (eclass, string);
-  if (!ev)
-    ev = g_enum_get_value_by_nick (eclass, string);
-
-  if (ev)
-    ret = ev->value;
-
-  g_type_class_unref (eclass);
+    *enum_value = value;
+  else
+    {
+      eclass = g_type_class_ref (type);
+      ev = g_enum_get_value_by_name (eclass, string);
+      if (!ev)
+	ev = g_enum_get_value_by_nick (eclass, string);
 
-  return ret;
+      if (ev)
+	*enum_value = ev->value;
+      else
+	{
+	  g_set_error (error,
+		       GTK_BUILDER_ERROR,
+		       GTK_BUILDER_ERROR_INVALID_VALUE,
+		       "Could not parse enum: `%s'",
+		       string);
+	  return FALSE;
+	}
+      
+      g_type_class_unref (eclass);
+    }
+  
+  return TRUE;
 }
 
-guint
-_gtk_builder_flags_from_string (GType        type, 
-                                const gchar *string)
+gboolean
+_gtk_builder_flags_from_string (GType         type, 
+                                const gchar  *string,
+				gint         *flags_value,
+				GError      **error)
 {
   GFlagsClass *fclass;
   gchar *endptr, *prevptr;
-  guint i, j, ret;
+  guint i, j, ret, value;
   gchar *flagstr;
   GFlagsValue *fv;
   const gchar *flag;
@@ -1240,70 +1273,85 @@ _gtk_builder_flags_from_string (GType        type,
   g_return_val_if_fail (G_TYPE_IS_FLAGS (type), 0);
   g_return_val_if_fail (string != 0, 0);
 
-  ret = strtoul (string, &endptr, 0);
+  ret = TRUE;
+  
+  value = strtoul (string, &endptr, 0);
   if (endptr != string) /* parsed a number */
-    return ret;
-
-  fclass = g_type_class_ref (type);
-
-  flagstr = g_strdup (string);
-  for (ret = i = j = 0; ; i++)
+    *flags_value = value;
+  else
     {
+      fclass = g_type_class_ref (type);
 
-      eos = flagstr[i] == '\0';
-
-      if (!eos && flagstr[i] != '|')
-        continue;
-
-      flag = &flagstr[j];
-      endptr = &flagstr[i];
-
-      if (!eos)
-        {
-          flagstr[i++] = '\0';
-          j = i;
-        }
-
-      /* trim spaces */
-      for (;;)
-        {
-          ch = g_utf8_get_char (flag);
-          if (!g_unichar_isspace (ch))
-            break;
-          flag = g_utf8_next_char (flag);
-        }
-
-      while (endptr > flag)
-        {
-          prevptr = g_utf8_prev_char (endptr);
-          ch = g_utf8_get_char (prevptr);
-          if (!g_unichar_isspace (ch))
-            break;
-          endptr = prevptr;
-        }
-
-      if (endptr > flag)
-        {
-          *endptr = '\0';
-          fv = g_flags_get_value_by_name (fclass, flag);
-
-          if (!fv)
-            fv = g_flags_get_value_by_nick (fclass, flag);
-
-          if (fv)
-            ret |= fv->value;
-          else
-            g_warning ("Unknown flag: '%s'", flag);
-        }
-
-      if (eos)
-        break;
+      flagstr = g_strdup (string);
+      for (value = i = j = 0; ; i++)
+	{
+	  
+	  eos = flagstr[i] == '\0';
+	  
+	  if (!eos && flagstr[i] != '|')
+	    continue;
+	  
+	  flag = &flagstr[j];
+	  endptr = &flagstr[i];
+	  
+	  if (!eos)
+	    {
+	      flagstr[i++] = '\0';
+	      j = i;
+	    }
+	  
+	  /* trim spaces */
+	  for (;;)
+	    {
+	      ch = g_utf8_get_char (flag);
+	      if (!g_unichar_isspace (ch))
+		break;
+	      flag = g_utf8_next_char (flag);
+	    }
+	  
+	  while (endptr > flag)
+	    {
+	      prevptr = g_utf8_prev_char (endptr);
+	      ch = g_utf8_get_char (prevptr);
+	      if (!g_unichar_isspace (ch))
+		break;
+	      endptr = prevptr;
+	    }
+	  
+	  if (endptr > flag)
+	    {
+	      *endptr = '\0';
+	      fv = g_flags_get_value_by_name (fclass, flag);
+	      
+	      if (!fv)
+		fv = g_flags_get_value_by_nick (fclass, flag);
+	      
+	      if (fv)
+		value |= fv->value;
+	      else
+		{
+		  g_set_error (error,
+			       GTK_BUILDER_ERROR,
+			       GTK_BUILDER_ERROR_INVALID_VALUE,
+			       "Unknown flag: `%s'",
+			       flag);
+		  ret = FALSE;
+		  break;
+		}
+	    }
+	  
+	  if (eos)
+	    {
+	      *flags_value = value;
+	      break;
+	    }
+	}
+      
+      g_free (flagstr);
+      
+      g_type_class_unref (fclass);
     }
 
-  g_free (flagstr);
-
-  g_type_class_unref (fclass);
-
   return ret;
 }
 
diff --git a/gtk/gtkbuilder.h b/gtk/gtkbuilder.h
index 0886d2a2d1..732acaa15e 100644
--- a/gtk/gtkbuilder.h
+++ b/gtk/gtkbuilder.h
@@ -117,8 +117,6 @@ gboolean     gtk_builder_value_from_string_type  (GtkBuilder    *builder,
                                                   const gchar  	*string,
                                                   GValue       	*value,
 						  GError       **error);
-guint        _gtk_builder_flags_from_string      (GType      	 type,
-						  const char  	*string);
 
 #define GTK_BUILDER_WARN_INVALID_CHILD_TYPE(object, type) \
   g_warning ("'%s' is not a valid child type of '%s'", type, g_type_name (G_OBJECT_TYPE (type)))
diff --git a/gtk/gtkbuilderparser.c b/gtk/gtkbuilderparser.c
index a1e31a1187..2996ac8a4f 100644
--- a/gtk/gtkbuilderparser.c
+++ b/gtk/gtkbuilderparser.c
@@ -158,12 +158,11 @@ error_missing_property_value (ParserData *data,
 }
 
 gboolean
-_gtk_builder_parse_boolean (const gchar  *string,
-			    gboolean     *value,
-			    GError      **error)
+_gtk_builder_boolean_from_string (const gchar  *string,
+				  gboolean     *value,
+				  GError      **error)
 {
   gboolean retval = TRUE;
-  int i;
   int length;
   
   g_assert (string != NULL);
@@ -405,7 +404,8 @@ parse_property (ParserData   *data,
         name = g_strdelimit (g_strdup (values[i]), "_", '-');
       else if (strcmp (names[i], "translatable") == 0)
 	{
-	  if (!_gtk_builder_parse_boolean (values[i], &translatable, error))
+	  if (!_gtk_builder_boolean_from_string (values[i], &translatable,
+						 error))
 	    return;
 	}
       else
@@ -463,12 +463,12 @@ parse_signal (ParserData   *data,
         handler = g_strdup (values[i]);
       else if (strcmp (names[i], "after") == 0)
 	{
-	  if (!_gtk_builder_parse_boolean (values[i], &after, error))
+	  if (!_gtk_builder_boolean_from_string (values[i], &after, error))
 	    return;
 	}
       else if (strcmp (names[i], "swapped") == 0)
 	{
-	  if (!_gtk_builder_parse_boolean (values[i], &swapped, error))
+	  if (!_gtk_builder_boolean_from_string (values[i], &swapped, error))
 	    return;
 	  swapped_set = TRUE;
 	}
diff --git a/gtk/gtkbuilderprivate.h b/gtk/gtkbuilderprivate.h
index 11d8974a77..70c9f7941f 100644
--- a/gtk/gtkbuilderprivate.h
+++ b/gtk/gtkbuilderprivate.h
@@ -107,8 +107,12 @@ void      _gtk_builder_add (GtkBuilder *builder,
 void      _gtk_builder_finish (GtkBuilder *builder);
 void _free_signal_info (SignalInfo *info,
                         gpointer user_data);
-gboolean _gtk_builder_parse_boolean (const gchar  *string,
-				     gboolean     *value,
-				     GError      **error);
+gboolean _gtk_builder_boolean_from_string (const gchar  *string,
+					   gboolean     *value,
+					   GError      **error);
+gboolean  _gtk_builder_flags_from_string (GType       type,
+					  const char *string,
+					  gint       *value,
+					  GError    **error);
 
 #endif /* __GTK_BUILDER_PRIVATE_H__ */
diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c
index 2a54b6e9e3..25fcb5f1fe 100644
--- a/gtk/gtkwidget.c
+++ b/gtk/gtkwidget.c
@@ -53,6 +53,7 @@
 #include "gtktooltip.h"
 #include "gtkinvisible.h"
 #include "gtkbuildable.h"
+#include "gtkbuilderprivate.h"
 #include "gtkalias.h"
 
 #define WIDGET_CLASS(w)	 GTK_WIDGET_GET_CLASS (w)
@@ -8517,7 +8518,13 @@ accel_group_start_element (GMarkupParseContext *context,
       if (strcmp (names[i], "key") == 0)
 	key = gdk_keyval_from_name (values[i]);
       else if (strcmp (names[i], "modifiers") == 0)
-	modifiers = _gtk_builder_flags_from_string (GDK_TYPE_MODIFIER_TYPE, values[i]);
+	{
+	  if (!_gtk_builder_flags_from_string (GDK_TYPE_MODIFIER_TYPE,
+					       values[i],
+					       &modifiers,
+					       error))
+	      return;
+	}
       else if (strcmp (names[i], "signal") == 0)
 	signal = g_strdup (values[i]);
     }