Re: misc. properties

[Owen Taylor <otaylor redhat com>]

Michael Meeks <michael ximian com> writes:

> On 10 Oct 2001, Owen Taylor wrote:
> > Some comments on the patch below. Other than that looks fine to
> > commit.
> 
>         Ok, fixed up and committed; here is the next patch:
> 
>         I'm not convinced about the 'history' property - since, unless the
> widget has it's children already registered it will not in fact set the
> history correctly - whether this is worth 'fixing' by effecting the
> history setting at a later time or something I don't know; also the name
> 'history' is rather curious for this property but ...

I'm not convinced either. It doesn't seem like it would work properly
used at construct time or generically. Yes, it might be a little 
cleaner to replace:

 get_history()
 ::changed

but I'm not sure it's so much cleaner that it's worth the change.
It's basically a "non-buildable" property, but we don't have any
way of flagging such, so it seems that it would show up in
GUI builders and not work without special casing.
 
>         Also, the 'text' property;

Your 'text' property is missing change notification. You need
to notify it when the widget emits ::changed on itself.

Other than that, the 'text' propery looks OK to commit.

Regards,
                                        Owen
 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/gtk+/ChangeLog,v
> retrieving revision 1.2378
> diff -u -r1.2378 ChangeLog
> --- ChangeLog	2001/10/11 20:37:50	1.2378
> +++ ChangeLog	2001/10/12 03:34:43
> @@ -1,3 +1,20 @@
> +2001-10-13  Michael Meeks  <michael ximian com>
> +
> +	* gdk/x11/gdkim-x11.c (_gdk_x11_initialize_locale): kill
> +	redundant return.
> +
> +	* gtk/gtkentry.c (gtk_entry_set_property),
> +	(gtk_entry_get_property): impl 'text'
> +	(gtk_entry_class_init): add the 'text' prop.
> +
> +2001-10-12  Michael Meeks  <michael ximian com>
> +
> +	* gtk/gtkoptionmenu.c (gtk_option_menu_set_property),
> +	(gtk_option_menu_get_property): impl.
> +	(gtk_option_menu_class_init): hook up the 'history'
> +	property.
> +	(gtk_option_menu_set_history): add notification.
> +
>  2001-10-11  Matthias Clasen  <matthiasc poet de>
> 
>  	* gtk/gtkcolorsel.c, gtk/gtkwidget.c, gtk/gtktreeview.c:
> Index: gdk/x11/gdkim-x11.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk/x11/gdkim-x11.c,v
> retrieving revision 1.25
> diff -u -r1.25 gdkim-x11.c
> --- gdk/x11/gdkim-x11.c	2001/10/10 21:54:25	1.25
> +++ gdk/x11/gdkim-x11.c	2001/10/12 03:34:43
> @@ -93,8 +93,6 @@
>    GDK_NOTE (XIM,
>  	    g_message ("%s multi-byte string functions.",
>  		       gdk_use_mb ? "Using" : "Not using"));
> -
> -  return current_locale;
>  }
> 
>  gchar*
> Index: gtk/gtkentry.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkentry.c,v
> retrieving revision 1.154
> diff -u -r1.154 gtkentry.c
> --- gtk/gtkentry.c	2001/10/03 21:50:57	1.154
> +++ gtk/gtkentry.c	2001/10/12 03:34:44
> @@ -78,7 +78,8 @@
>    PROP_INVISIBLE_CHAR,
>    PROP_ACTIVATES_DEFAULT,
>    PROP_WIDTH_CHARS,
> -  PROP_SCROLL_OFFSET
> +  PROP_SCROLL_OFFSET,
> +  PROP_TEXT
>  };
> 
>  static guint signals[LAST_SIGNAL] = { 0 };
> @@ -465,7 +466,6 @@
>                                                       _("Number of characters to leave space for in the entry."),
>                                                       -1,
>                                                       G_MAXINT,
> -
>                                                       -1,
>                                                       G_PARAM_READABLE | G_PARAM_WRITABLE));
> 
> @@ -476,9 +476,16 @@
>                                                       _("Number of pixels of the entry scrolled off the screen to the left"),
>                                                       0,
>                                                       G_MAXINT,
> -
>                                                       0,
>                                                       G_PARAM_READABLE));
> +
> +  g_object_class_install_property (gobject_class,
> +                                   PROP_TEXT,
> +                                   g_param_spec_string ("text",
> +							_("Text"),
> +							_("The contents of the entry"),
> +							"",
> +							G_PARAM_READABLE | G_PARAM_WRITABLE));
> 
>    gtk_widget_class_install_style_property (widget_class,
>  					   g_param_spec_boxed ("cursor_color",
> @@ -809,6 +816,10 @@
>        gtk_entry_set_width_chars (entry, g_value_get_int (value));
>        break;
> 
> +    case PROP_TEXT:
> +      gtk_entry_set_text (entry, g_value_get_string (value));
> +      break;
> +
>      case PROP_SCROLL_OFFSET:
>      default:
>        G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> @@ -854,6 +865,9 @@
>        break;
>      case PROP_SCROLL_OFFSET:
>        g_value_set_int (value, entry->scroll_offset);
> +      break;
> +    case PROP_TEXT:
> +      g_value_set_string (value, gtk_entry_get_text (entry));
>        break;
> 
>      default: