Re: [gtk-list] Re: new signal GtkWidget::user_signal?

[Owen Taylor <owt1 cornell edu>]

Tim Janik <Tim.Janik@Hamburg.Netsurf.DE> writes:

> > > hello everybody,
> > > 
> > > i'd like to have an additional signal supported by the GtkWidget base class.
> > > 
> > > GtkWidget:user_signal
> > > 
> > > this signal will be invoked by the application programmer and not inside the
> > > toolkit.
> > 
> > I must admit that my initial reaction to this was "ugh".
> 
> well i expected reactions like this (especially from you ;)

It's a hard job, but someone has to do it. ;-)

> > There does seem to be a need (or at least a perceived need) for
> > this type of functionality.
> > 
> > - people want to build "pseudo-widgets" without going through
> >   all the trouble of building a real widget, and add new signals
> >   to them.
> > 
> > - writers of language bindings want to be able to derive
> >   new widget types and add signals without having to extend
> >   the class structure.
> 
> exactly the point!
> 
> > Your facility to some extent does meet these requirements, but
> > not very gracefully, since the resulting signal will be quite
> > non-standard, from the users point of view.
> 
> i don't quite know what you mean by "non-standard" here...

It doesn't look like a normal signal. You have to switch on
an enumeration. You can't specify different parameters.

> > Perhaps we should instead look at how to enable the user
> > to add new signals without modifying the widget.
> > 
> > gint
> > gtk_signal_new (const gchar         *name,
> > 		GtkSignalRunType     run_type,
> > 		gint                 object_type,
> > 		gint                 function_offset,
> > 		GtkSignalMarshaller  marshaller,
> > 		GtkType              return_val,
> > 		gint                 nparams,
> > 		...)
> 
> the function_offset needs to be 0 anyways, because there
> will be no possibility to have an additional slot in the widget class
> that could be (miss-)used as a user signal default handler. also
> gtk_signal_new() relies on previously reserved space in the
> gint *signals; array in _GtkObjectClass. so this absolutly
> disqualifies gtk_signal_new() for user defined signals.

Errr. Take a look at gtk_object_class_add_signals.  There is a realloc
if you add signals later, but that is all.

  gint signal;

  signal = gtk_signal_new ("my_user_signal", GTK_RUN_LAST,
		  gtk_button_get_type(), 0, gtk_signal_default_marshaller,
		  GTK_TYPE_NONE, 0, NULL);

  gtk_object_class_add_signals ( gtk_type_class (gtk_button_get_type ()),
				 &signal, 1);

works fine for me.

> > The problem here, is function_offset. But since this
> > can never be zero, why not make function_offset = 0 =>
> > no default handler.
> > 
> > What if you do want a default handler? In that case
> > we'd to provide an alternative interface. But this only would
> > come up when defining new widget types, since the default
> > handler is per widget class, not per widget.
> 
> this is besides the point. i'm not talking about a dynamically
> growing (or even shrinkable) signal stack (per class) here. my
> initial idea wasn't even about user signals per widget class,
> but on a per widget basis. as Gtk's internal widget code probably
> can't know about the signal cause it's user defined, it should
> have been implied that a default handler is out of scope here.

There doesn't seem to be much point for a default handler
certainly - that functionality is pretty much only useful
for inheritance.

> > A few comments on the original proposal:
> > 
> > > it can be invoked with a call to
> > > 
> > > void	gtk_widget_emit_user_signal (GtkWidget *widget,
> > > 				     gint       user_type,
> > > 				     gpointer   signal_data);
> > 
> > Why not simply:
> > 
> > gtk_widget_emit_by_name (widget, "user_data", user_type, signal_data)
> 
> hm because there would be only be the "user_data" signal that could be
> emitted, so why do you want to have this specified each time
> the function is called and even checked with a
> g_return_if_fail (g_str_equal (signal_name, "user_data)); ?

That should have been:

  gtk_signal_emit_by_name

maybe that obscured the point that the normal "emit" mechanism
was being used. 

> > OK, it isn't type safe, but neither is anything else about
> > the existing signal system.
> 
> well, as good as C might be this is certainly one of its drawbacks,
> and not caused by bad design on the signal code.

Quite true.

> > [...]

> > There is the typical "namespace registration" problem here -
> > if two different entities use the same value for the same widget
> > there will be problems.
> > 
> > If you defined a new signal type (or specified user signal types
> > by string - which might be preferable), then this would be
> > less of a problem.
> 
> as i wrote in a previous mail to nathan, this would cause some bigger
> changes in the signal code, wich is not what i had in mind i the first
> place.

Two lines. Two lines. (Included below for demo purposes)

> but at this point i think i must admitt, that i wasn't so sure about the
> best way of implementing this in the first place as well. so i just threw
> in the idea to see what people are going to make out of it (this does not
> imply that there wouldn't be a need for user defined signals on my part).

I'm not denying the need. But I think they should be keyed by 
string. If you think the above code fragment is too complicated
for the average user, I suppose we could provide

  gtk_signal_new_simple ("my_user_signal", gtk_button_get_type());

Which automatically uses the default marshaller and adds the result
to the class signals of the specified type.

Regards,
                                        Owen

diff -u -r1.6 gtksignal.c
--- gtksignal.c	1998/01/25 18:45:56	1.6
+++ gtksignal.c	1998/01/27 18:22:40
@@ -871,7 +871,7 @@
       gtk_emission_add (&current_emissions, object, signal_type);
 
     restart:
-      if (GTK_RUN_TYPE (signal->run_type) != GTK_RUN_LAST)
+      if (GTK_RUN_TYPE (signal->run_type) != GTK_RUN_LAST && signal->function_offset != 0)
 	{
 	  signal_func_offset = (guchar**) ((guchar*) object->klass + signal->function_offset);
 	  if (*signal_func_offset)
@@ -898,7 +898,7 @@
 	  goto restart;
 	}
 
-      if (GTK_RUN_TYPE (signal->run_type) != GTK_RUN_FIRST)
+      if (GTK_RUN_TYPE (signal->run_type) != GTK_RUN_FIRST && signal->function_offset != 0)
 	{
 	  signal_func_offset = (guchar**) ((guchar*) object->klass + signal->function_offset);
 	  if (*signal_func_offset)