Re: gnome-libs property changes, property notification

[Martin Baulig <martin home-of-linux org>]

Hi Tim,

> i read over recent gnome-libs and basically have to say the new property stuff
> you did looks very good, i just cought a few minor things ;)

Cool !

> * instead of x = g_strdup (g_value_get_string (value)) you can use
>   x = g_value_dup_string (value)
> * code like
>   - case ARG_FONTSET:
>   + case PROP_FONTSET:
>       if (text->font)
>         gdk_font_unref (text->font);
>   -   text->font = gdk_fontset_load (GTK_VALUE_STRING (*arg));
>   +   text->font = gdk_fontset_load (g_value_get_string (value));
>   should account for g_value_get_string (value) possibly being NULL (old bug,
>   the same held true for GTK_VALUE_STRING (*arg))
> * you introduced a few leaks:
>   case PROP_TEXT:
>     g_value_set_string (value, g_strdup (text->text));
>     break;
>   g_value_set_string() will already duplicate the string internally (you don't
>   need to add a reference count to an obj in g_value_set_object (val, obj) either)
> * the casting macros work only on real objects, not NULL objects, so code like
>   - obj = GTK_VALUE_OBJECT (*arg);
>   + obj = GTK_OBJECT (g_value_get_object (value));
>     if (obj) { ...
>   should cast only after you know g_value_get_object (value) is non-NULL (the
>   cast in this specific case is superfluous btw)

This should now be fixed.

> another note on notification. in 2.0 with object properties, we're emitting
> a notification signal for each property that has changed on an object, the
> signal's detail holds the property name. that means, code like:

Hmm, I have a little problem with this notification stuff:

Currently, some/most of the set_property() handlers call the accessor function
to modify things. When I now add that notify call in the accessor function,
doesn't this result in a duplicate notification if the accessor func is called
from the set_property() handler ?

-- 
Martin Baulig
martin gnome org (private)
baulig suse de (work)