diff options
author | Juan Pablo Ugarte <juanpablougarte@gmail.com> | 2014-04-29 16:22:32 -0300 |
---|---|---|
committer | Juan Pablo Ugarte <juanpablougarte@gmail.com> | 2014-05-01 17:59:53 -0300 |
commit | 49fa04212b644f19ce576a8011e3f4fcda6a0806 (patch) | |
tree | bcd193c9b4c61aff1638208d5a1e517a326af6db | |
parent | c4afca906c2ea02ae7a261d2e7e0c8902f3f09e5 (diff) | |
download | gtk+-49fa04212b644f19ce576a8011e3f4fcda6a0806.tar.gz |
GtkBuilder: improved parsing error report for invalid properties and signals.
Added GTK_BUILDER_ERROR_INVALID_PROPERTY and GTK_BUILDER_ERROR_INVALID_SIGNAL
error codes
ObjectInfo: Use a GType instead of a char * for the class name.
PropertyInfo: Use a GParamSpec instead of a char * for the property name.
SignalInfo: Use signal id and detail quark instead of a detailed signal name string.
This not only save us a few malloc in each case but lets us simplify the code
and report unknown properties and signals as a parsing error instead of just
printing a warning.
-rw-r--r-- | gtk/gtkbuilder.c | 136 | ||||
-rw-r--r-- | gtk/gtkbuilder.h | 6 | ||||
-rw-r--r-- | gtk/gtkbuilderparser.c | 104 | ||||
-rw-r--r-- | gtk/gtkbuilderprivate.h | 9 | ||||
-rw-r--r-- | testsuite/gtk/builder.c | 14 |
5 files changed, 147 insertions, 122 deletions
diff --git a/gtk/gtkbuilder.c b/gtk/gtkbuilder.c index d6a435f16b..165e9e5269 100644 --- a/gtk/gtkbuilder.c +++ b/gtk/gtkbuilder.c @@ -436,7 +436,7 @@ gtk_builder_real_get_type_from_name (GtkBuilder *builder, typedef struct { gchar *object; - gchar *name; + GParamSpec *pspec; gchar *value; } DelayedProperty; @@ -450,13 +450,8 @@ gtk_builder_get_parameters (GtkBuilder *builder, GArray **filtered_parameters) { GSList *l; - GParamSpec *pspec; - GObjectClass *oclass; DelayedProperty *property; GError *error = NULL; - - oclass = g_type_class_ref (object_type); - g_assert (oclass != NULL); if (parameters) *parameters = g_array_new (FALSE, FALSE, sizeof (GParameter)); @@ -468,19 +463,10 @@ gtk_builder_get_parameters (GtkBuilder *builder, PropertyInfo *prop = (PropertyInfo*)l->data; GParameter parameter = { NULL }; - pspec = g_object_class_find_property (G_OBJECT_CLASS (oclass), - prop->name); - if (!pspec) - { - g_warning ("Unknown property: %s.%s", - g_type_name (object_type), prop->name); - continue; - } - - parameter.name = prop->name; + parameter.name = prop->pspec->name; - if (G_IS_PARAM_SPEC_OBJECT (pspec) && - (G_PARAM_SPEC_VALUE_TYPE (pspec) != GDK_TYPE_PIXBUF)) + if (G_IS_PARAM_SPEC_OBJECT (prop->pspec) && + (G_PARAM_SPEC_VALUE_TYPE (prop->pspec) != GDK_TYPE_PIXBUF)) { GObject *object = gtk_builder_get_object (builder, prop->data); @@ -491,17 +477,17 @@ gtk_builder_get_parameters (GtkBuilder *builder, } else { - if (pspec->flags & G_PARAM_CONSTRUCT_ONLY) + if (prop->pspec->flags & G_PARAM_CONSTRUCT_ONLY) { g_warning ("Failed to get construct only property " "%s of %s with value `%s'", - prop->name, object_name, prop->data); + prop->pspec->name, object_name, prop->data); continue; } /* Delay setting property */ property = g_slice_new (DelayedProperty); + property->pspec = prop->pspec; property->object = g_strdup (object_name); - property->name = g_strdup (prop->name); property->value = g_strdup (prop->data); builder->priv->delayed_properties = g_slist_prepend (builder->priv->delayed_properties, property); @@ -515,18 +501,18 @@ gtk_builder_get_parameters (GtkBuilder *builder, */ continue; } - else if (!gtk_builder_value_from_string (builder, pspec, + else if (!gtk_builder_value_from_string (builder, prop->pspec, prop->data, ¶meter.value, &error)) { g_warning ("Failed to set property %s.%s to %s: %s", - g_type_name (object_type), prop->name, prop->data, + g_type_name (object_type), prop->pspec->name, prop->data, error->message); g_error_free (error); error = NULL; continue; } - if (pspec->flags & filter_flags) + if (prop->pspec->flags & filter_flags) { if (filtered_parameters) g_array_append_val (*filtered_parameters, parameter); @@ -537,8 +523,6 @@ gtk_builder_get_parameters (GtkBuilder *builder, g_array_append_val (*parameters, parameter); } } - - g_type_class_unref (oclass); } static GObject * @@ -628,7 +612,6 @@ _gtk_builder_construct (GtkBuilder *builder, GError **error) { GArray *parameters, *construct_parameters; - GType object_type; GObject *obj; int i; GtkBuildableIface *iface; @@ -636,26 +619,17 @@ _gtk_builder_construct (GtkBuilder *builder, GtkBuildable *buildable; GParamFlags param_filter_flags; - g_assert (info->class_name != NULL); - object_type = gtk_builder_get_type_from_name (builder, info->class_name); - if (object_type == G_TYPE_INVALID) - { - g_set_error (error, - GTK_BUILDER_ERROR, - GTK_BUILDER_ERROR_INVALID_VALUE, - "Invalid object type `%s'", - info->class_name); - return NULL; - } - else if (builder->priv->template_type != 0 && - g_type_is_a (object_type, builder->priv->template_type)) + g_assert (info->type != G_TYPE_INVALID); + + if (builder->priv->template_type != 0 && + g_type_is_a (info->type, builder->priv->template_type)) { g_set_error (error, GTK_BUILDER_ERROR, GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED, "Refused to build object of type `%s' because it " "conforms to the template type `%s', avoiding infinite recursion.", - info->class_name, g_type_name (builder->priv->template_type)); + g_type_name (info->type), g_type_name (builder->priv->template_type)); return NULL; } @@ -676,7 +650,7 @@ _gtk_builder_construct (GtkBuilder *builder, else param_filter_flags = G_PARAM_CONSTRUCT | G_PARAM_CONSTRUCT_ONLY; - gtk_builder_get_parameters (builder, object_type, + gtk_builder_get_parameters (builder, info->type, info->id, info->properties, param_filter_flags, @@ -723,7 +697,7 @@ _gtk_builder_construct (GtkBuilder *builder, } else { - obj = g_object_newv (object_type, + obj = g_object_newv (info->type, construct_parameters->len, (GParameter *)construct_parameters->data); @@ -740,7 +714,7 @@ _gtk_builder_construct (GtkBuilder *builder, g_object_ref_sink (obj); GTK_NOTE (BUILDER, - g_print ("created %s of type %s\n", info->id, info->class_name)); + g_print ("created %s of type %s\n", info->id, g_type_name (info->type))); for (i = 0; i < construct_parameters->len; i++) { @@ -800,18 +774,16 @@ _gtk_builder_apply_properties (GtkBuilder *builder, GError **error) { GArray *parameters; - GType object_type; GtkBuildableIface *iface; GtkBuildable *buildable; gboolean custom_set_property; gint i; g_assert (info->object != NULL); - g_assert (info->class_name != NULL); - object_type = gtk_builder_get_type_from_name (builder, info->class_name); + g_assert (info->type != G_TYPE_INVALID); /* Fetch all properties that are not construct-only */ - gtk_builder_get_parameters (builder, object_type, + gtk_builder_get_parameters (builder, info->type, info->id, info->properties, G_PARAM_CONSTRUCT_ONLY, @@ -903,11 +875,6 @@ static void gtk_builder_apply_delayed_properties (GtkBuilder *builder) { GSList *l, *props; - DelayedProperty *property; - GObject *object; - GType object_type; - GObjectClass *oclass; - GParamSpec *pspec; /* take the list over from the builder->priv. * @@ -919,36 +886,22 @@ gtk_builder_apply_delayed_properties (GtkBuilder *builder) for (l = props; l; l = l->next) { - property = (DelayedProperty*)l->data; + DelayedProperty *property = l->data; + GObject *object, *obj; + object = g_hash_table_lookup (builder->priv->objects, property->object); g_assert (object != NULL); - object_type = G_OBJECT_TYPE (object); - g_assert (object_type != G_TYPE_INVALID); - - oclass = g_type_class_ref (object_type); - g_assert (oclass != NULL); - - pspec = g_object_class_find_property (G_OBJECT_CLASS (oclass), - property->name); - if (!pspec) - g_warning ("Unknown property: %s.%s", g_type_name (object_type), - property->name); + obj = g_hash_table_lookup (builder->priv->objects, property->value); + if (obj) + g_object_set (object, property->pspec->name, obj, NULL); else - { - GObject *obj; + g_warning ("No object called: %s", property->value); + - obj = g_hash_table_lookup (builder->priv->objects, property->value); - if (!obj) - g_warning ("No object called: %s", property->value); - else - g_object_set (object, property->name, obj, NULL); - } g_free (property->value); g_free (property->object); - g_free (property->name); g_slice_free (DelayedProperty, property); - g_type_class_unref (oclass); } g_slist_free (props); } @@ -957,7 +910,7 @@ static inline void free_binding_info (gpointer data, gpointer user) { BindingInfo *info = data; - g_free (info->target_property); + g_free (info->source); g_free (info->source_property); g_slice_free (BindingInfo, data); @@ -975,7 +928,7 @@ gtk_builder_create_bindings (GtkBuilder *builder) if ((source = gtk_builder_get_object (builder, info->source))) g_object_bind_property (source, info->source_property, - info->target, info->target_property, + info->target, info->target_pspec->name, info->flags); else g_warning ("Could not find source object '%s' to bind property '%s'", @@ -1724,6 +1677,7 @@ gtk_builder_connect_signals_full (GtkBuilder *builder, GSList *l; GObject *object; GObject *connect_object; + GString *detailed_id = NULL; g_return_if_fail (GTK_IS_BUILDER (builder)); g_return_if_fail (func != NULL); @@ -1735,9 +1689,12 @@ gtk_builder_connect_signals_full (GtkBuilder *builder, for (l = builder->priv->signals; l; l = l->next) { SignalInfo *signal = (SignalInfo*)l->data; + const gchar *signal_name; g_assert (signal != NULL); - g_assert (signal->name != NULL); + g_assert (signal->id != 0); + + signal_name = g_signal_name (signal->id); object = g_hash_table_lookup (builder->priv->objects, signal->object_name); @@ -1751,17 +1708,30 @@ gtk_builder_connect_signals_full (GtkBuilder *builder, signal->connect_object_name); if (!connect_object) g_warning ("Could not lookup object %s on signal %s of object %s", - signal->connect_object_name, signal->name, - signal->object_name); + signal->connect_object_name, signal_name, + signal->object_name); } - - func (builder, object, signal->name, signal->handler, - connect_object, signal->flags, user_data); + + if (signal->detail) + { + if (detailed_id == NULL) + detailed_id = g_string_new (""); + + g_string_printf (detailed_id, "%s::%s", signal_name, + g_quark_to_string (signal->detail)); + signal_name = detailed_id->str; + } + + func (builder, object, signal_name, signal->handler, + connect_object, signal->flags, user_data); } g_slist_foreach (builder->priv->signals, (GFunc)_free_signal_info, NULL); g_slist_free (builder->priv->signals); builder->priv->signals = NULL; + + if (detailed_id) + g_string_free (detailed_id, TRUE); } /** diff --git a/gtk/gtkbuilder.h b/gtk/gtkbuilder.h index a548229eb1..12815d2f0a 100644 --- a/gtk/gtkbuilder.h +++ b/gtk/gtkbuilder.h @@ -62,6 +62,8 @@ typedef struct _GtkBuilderPrivate GtkBuilderPrivate; * @GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED: A specified object type is of the same type or * derived from the type of the composite class being extended with builder XML. * @GTK_BUILDER_ERROR_TEMPLATE_MISMATCH: The wrong type was specified in a composite class’s template XML + * @GTK_BUILDER_ERROR_INVALID_PROPERTY: The specified property is unknown for the object class. + * @GTK_BUILDER_ERROR_INVALID_SIGNAL: The specified signal is unknown for the object class. * * Error codes that identify various errors that can occur while using * #GtkBuilder. @@ -78,7 +80,9 @@ typedef enum GTK_BUILDER_ERROR_VERSION_MISMATCH, GTK_BUILDER_ERROR_DUPLICATE_ID, GTK_BUILDER_ERROR_OBJECT_TYPE_REFUSED, - GTK_BUILDER_ERROR_TEMPLATE_MISMATCH + GTK_BUILDER_ERROR_TEMPLATE_MISMATCH, + GTK_BUILDER_ERROR_INVALID_PROPERTY, + GTK_BUILDER_ERROR_INVALID_SIGNAL } GtkBuilderError; GDK_AVAILABLE_IN_ALL diff --git a/gtk/gtkbuilderparser.c b/gtk/gtkbuilderparser.c index 16226c9ed2..c09cb880fb 100644 --- a/gtk/gtkbuilderparser.c +++ b/gtk/gtkbuilderparser.c @@ -216,24 +216,19 @@ builder_construct (ParserData *data, return object; } -static gchar * +static GType _get_type_by_symbol (const gchar* symbol) { static GModule *module = NULL; GTypeGetFunc func; - GType type; if (!module) module = g_module_open (NULL, 0); if (!g_module_symbol (module, symbol, (gpointer)&func)) - return NULL; + return G_TYPE_INVALID; - type = func (); - if (type == G_TYPE_INVALID) - return NULL; - - return g_strdup (g_type_name (type)); + return func (); } static void @@ -318,7 +313,7 @@ parse_object (GMarkupParseContext *context, { ObjectInfo *object_info; ChildInfo* child_info; - const gchar *object_class = NULL; + GType object_type = G_TYPE_INVALID; const gchar *object_id = NULL; const gchar *constructor = NULL; gchar *internal_id = NULL; @@ -334,19 +329,30 @@ parse_object (GMarkupParseContext *context, for (i = 0; names[i] != NULL; i++) { if (strcmp (names[i], "class") == 0) - object_class = values[i]; + { + object_type = gtk_builder_get_type_from_name (data->builder, values[i]); + + if (object_type == G_TYPE_INVALID) + { + g_markup_parse_context_get_position (context, &line, NULL); + g_set_error (error, GTK_BUILDER_ERROR, + GTK_BUILDER_ERROR_INVALID_VALUE, + _("Invalid object type `%s' on line %d"), + values[i], line); + return; + } + } else if (strcmp (names[i], "id") == 0) object_id = values[i]; else if (strcmp (names[i], "constructor") == 0) constructor = values[i]; else if (strcmp (names[i], "type-func") == 0) { - /* Call the GType function, and return the name of the GType, - * it's guaranteed afterwards that g_type_from_name on the name - * will return our GType - */ - object_class = _get_type_by_symbol (values[i]); - if (!object_class) + /* Call the GType function, and return the GType, it's guaranteed afterwards + * that g_type_from_name on the name will return our GType + */ + object_type = _get_type_by_symbol (values[i]); + if (object_type == G_TYPE_INVALID) { g_markup_parse_context_get_position (context, &line, NULL); g_set_error (error, GTK_BUILDER_ERROR, @@ -363,7 +369,7 @@ parse_object (GMarkupParseContext *context, } } - if (!object_class) + if (object_type == G_TYPE_INVALID) { error_missing_attribute (data, element_name, "class", error); return; @@ -398,7 +404,7 @@ parse_object (GMarkupParseContext *context, } object_info = g_slice_new0 (ObjectInfo); - object_info->class_name = g_strdup (object_class); + object_info->type = object_type; object_info->id = (internal_id) ? internal_id : g_strdup (object_id); object_info->constructor = g_strdup (constructor); state_push (data, object_info); @@ -486,7 +492,7 @@ parse_template (GMarkupParseContext *context, ++data->cur_object_level; object_info = g_slice_new0 (ObjectInfo); - object_info->class_name = g_strdup (object_class); + object_info->type = parsed_type; object_info->id = g_strdup (object_class); object_info->object = gtk_builder_get_object (data->builder, object_class); state_push (data, object_info); @@ -516,7 +522,6 @@ free_object_info (ObjectInfo *info) (GFunc)free_property_info, NULL); g_slist_free (info->properties); g_free (info->constructor); - g_free (info->class_name); g_free (info->id); g_slice_free (ObjectInfo, info); } @@ -584,14 +589,14 @@ parse_property (ParserData *data, GError **error) { PropertyInfo *info; - const gchar *name = NULL; const gchar *context = NULL; const gchar *bind_source = NULL; const gchar *bind_property = NULL; GBindingFlags bind_flags = G_BINDING_DEFAULT; gboolean translatable = FALSE; ObjectInfo *object_info; - int i; + GParamSpec *pspec = NULL; + gint i; object_info = state_peek_info (data, ObjectInfo); if (!object_info || @@ -605,7 +610,26 @@ parse_property (ParserData *data, for (i = 0; names[i] != NULL; i++) { if (strcmp (names[i], "name") == 0) - name = values[i]; + { + GObjectClass *oclass = g_type_class_ref (object_info->type); + gchar *name = g_strdelimit (g_strdup (values[i]), "_", '-'); + + g_assert (oclass != NULL); + pspec = g_object_class_find_property (oclass, name); + g_type_class_unref (oclass); + g_free (name); + + if (!pspec) + { + gint line; + g_markup_parse_context_get_position (data->ctx, &line, NULL); + g_set_error (error, GTK_BUILDER_ERROR, + GTK_BUILDER_ERROR_INVALID_PROPERTY, + _("Invalid property: %s.%s on line %d"), + g_type_name (object_info->type), values[i], line); + return; + } + } else if (strcmp (names[i], "translatable") == 0) { if (!_gtk_builder_boolean_from_string (values[i], &translatable, @@ -641,7 +665,7 @@ parse_property (ParserData *data, } } - if (!name) + if (!pspec) { error_missing_attribute (data, element_name, "name", error); return; @@ -651,7 +675,7 @@ parse_property (ParserData *data, { BindingInfo *binfo = g_slice_new0 (BindingInfo); - binfo->target_property = g_strdup (name); + binfo->target_pspec = pspec; binfo->source = g_strdup (bind_source); binfo->source_property = g_strdup (bind_property); binfo->flags = bind_flags; @@ -667,9 +691,9 @@ parse_property (ParserData *data, } info = g_slice_new0 (PropertyInfo); - info->name = g_strdelimit (g_strdup (name), "_", '-'); + info->pspec = pspec; info->translatable = translatable; - info->bound = (bind_source != NULL && bind_property != NULL); + info->bound = (bind_source && bind_property); info->context = g_strdup (context); info->text = g_string_new (""); state_push (data, info); @@ -681,7 +705,6 @@ static void free_property_info (PropertyInfo *info) { g_free (info->data); - g_free (info->name); g_free (info->context); /* info->text is already freed */ g_slice_free (PropertyInfo, info); @@ -695,14 +718,15 @@ parse_signal (ParserData *data, GError **error) { SignalInfo *info; - const gchar *name = NULL; const gchar *handler = NULL; const gchar *object = NULL; gboolean after = FALSE; gboolean swapped = FALSE; gboolean swapped_set = FALSE; ObjectInfo *object_info; - int i; + guint id = 0; + GQuark detail = 0; + gint i; object_info = state_peek_info (data, ObjectInfo); if (!object_info || @@ -716,7 +740,19 @@ parse_signal (ParserData *data, for (i = 0; names[i] != NULL; i++) { if (strcmp (names[i], "name") == 0) - name = values[i]; + { + if (!g_signal_parse_name (values[i], object_info->type, + &id, &detail, FALSE)) + { + gint line; + g_markup_parse_context_get_position (data->ctx, &line, NULL); + g_set_error (error, GTK_BUILDER_ERROR, + GTK_BUILDER_ERROR_INVALID_SIGNAL, + _("Invalid signal `%s' for type `%s' on line %d"), + values[i], g_type_name (object_info->type), line); + return; + } + } else if (strcmp (names[i], "handler") == 0) handler = values[i]; else if (strcmp (names[i], "after") == 0) @@ -742,7 +778,7 @@ parse_signal (ParserData *data, } } - if (!name) + if (!id) { error_missing_attribute (data, element_name, "name", error); return; @@ -758,7 +794,8 @@ parse_signal (ParserData *data, swapped = TRUE; info = g_slice_new0 (SignalInfo); - info->name = g_strdup (name); + info->id = id; + info->detail = detail; info->handler = g_strdup (handler); if (after) info->flags |= G_CONNECT_AFTER; @@ -775,7 +812,6 @@ void _free_signal_info (SignalInfo *info, gpointer user_data) { - g_free (info->name); g_free (info->handler); g_free (info->connect_object_name); g_free (info->object_name); diff --git a/gtk/gtkbuilderprivate.h b/gtk/gtkbuilderprivate.h index 9941121c84..fe619d4974 100644 --- a/gtk/gtkbuilderprivate.h +++ b/gtk/gtkbuilderprivate.h @@ -31,7 +31,7 @@ typedef struct { typedef struct { TagInfo tag; - gchar *class_name; + GType type; gchar *id; gchar *constructor; GSList *properties; @@ -60,7 +60,7 @@ typedef struct { typedef struct { TagInfo tag; - gchar *name; + GParamSpec *pspec; GString *text; gchar *data; gboolean translatable:1; @@ -71,7 +71,8 @@ typedef struct { typedef struct { TagInfo tag; gchar *object_name; - gchar *name; + guint id; + GQuark detail; gchar *handler; GConnectFlags flags; gchar *connect_object_name; @@ -80,7 +81,7 @@ typedef struct { typedef struct { GObject *target; - gchar *target_property; + GParamSpec *target_pspec; gchar *source; gchar *source_property; GBindingFlags flags; diff --git a/testsuite/gtk/builder.c b/testsuite/gtk/builder.c index 74da1b3113..3d6f9e742a 100644 --- a/testsuite/gtk/builder.c +++ b/testsuite/gtk/builder.c @@ -131,6 +131,20 @@ test_parser (void) GTK_BUILDER_ERROR_DUPLICATE_ID)); g_error_free (error); + error = NULL; + gtk_builder_add_from_string (builder, "<interface><object class=\"GtkButton\" id=\"a\"><property name=\"deafbeef\"></property></object></interface>", -1, &error); + g_assert (g_error_matches (error, + GTK_BUILDER_ERROR, + GTK_BUILDER_ERROR_INVALID_PROPERTY)); + g_error_free (error); + + error = NULL; + gtk_builder_add_from_string (builder, "<interface><object class=\"GtkButton\" id=\"a\"><signal name=\"deafbeef\" handler=\"gtk_true\"/></object></interface>", -1, &error); + g_assert (g_error_matches (error, + GTK_BUILDER_ERROR, + GTK_BUILDER_ERROR_INVALID_SIGNAL)); + g_error_free (error); + g_object_unref (builder); } |