Re: Patches to gtkwidget and gtkwindow for property support

[Tim Janik <timj gtk org>]

On Sun, 18 Feb 2001, John Margaglione wrote:

> Here are my changes to GtkWidget and GtkWindow.  I converted the args to
> properties using g_type support.  Let me know if they are ok, and I will
> continue with the other files. The only open question I have is when to
> use g_object_notify.  Some of the files (gtktexttag.c) use it, and some
> (gtkcellrenderer.c) don't.

please see
http://mail.gnome.org/archives/gtk-devel-list/2000-December/msg00237.html
which gives a basic example on how to use g_object_notify().

apart from needing your patch in unified diff format, there are a couple
minor items that occoured to me when glancing over it.


>   g_object_class_install_property (gobject_class,
>                                    PROP_NAME,
>                                    g_param_spec_string ("name",
> 							_("Widget Name"),
>                                                         _("The name of the widget"),
>                                                         "",
>                                                         G_PARAM_READABLE | G_PARAM_WRITABLE));

you can use G_PARAM_READWRITE now as a synonym
for (G_PARAM_READABLE | G_PARAM_WRITABLE).

>   g_object_class_install_property (gobject_class,
>                                    PROP_X,
>                                    g_param_spec_int ("x",
> 						     _("X coordinate of window"),
> 						     _("The X-axis coordinate of the top-left corner of the widget"),
> 						     0,
> 						     G_MAXINT,
> 						     -1,
> 						     G_PARAM_READABLE | G_PARAM_WRITABLE));

the extra arguments for an Int param spec are minimum, maximum and
default, your default (-1) is smaller than the minimum value.

(ok, i'll put checks into glib for this since it's actually easy to catch)


>     case PROP_STYLE:
>       gtk_widget_set_style (widget, (GtkStyle*) g_value_get_as_pointer (value));

>       gtk_widget_set_extension_events (widget, 
> 				       (GdkExtensionMode) g_value_get_enum (value));

these two casts are uneccessary.


<     case ARG_APP_PAINTABLE:
<       GTK_VALUE_BOOL (*arg) = (GTK_WIDGET_APP_PAINTABLE (widget) != FALSE);

>     case PROP_APP_PAINTABLE:
>       g_value_set_boolean (value, (GTK_WIDGET_APP_PAINTABLE (widget) != FALSE));

the GTK_WIDGET_*() flags macros all return just TRUE or FALSE, so the
additional "!= FALSE" isn't necessary here. (yes i see it's there in
current code as well, it's still ueccessary ;)


>     case PROP_STYLE:
>       g_value_set_boxed (value, (gpointer) gtk_widget_get_style (widget));

sicne we don't have a literal GBoxed type but take a gconstpointer
instead, you don't need this cast either.


<   gtk_object_add_arg_type ("GtkWidget::events", GTK_TYPE_GDK_EVENT_MASK, GTK_ARG_READWRITE, ARG_EVENTS);

>   g_object_class_install_property (gobject_class,
>                                    PROP_EVENTS,
>                                    g_param_spec_int ("events",
>                                                    _("Events"),
>                                                    _("Widget events"),
>                                                    0,
>                                                    GDK_ALL_EVENTS_MASK,
>                                                    0,
>                                                    G_PARAM_READABLE | G_PARAM_WRITABLE));

here you changed the property type, that should not
happen. simply use g_param_spec_enum (,GTK_TYPE_GDK_EVENT_MASK,)
here.

i'll do a more detailed review when you send in a unified diff,
however in general the patch looks pretty good already, and i'm
very thankfull that someone's adressing the GtkArg->GParam transition.

> 
> John

---
ciaoTJ