Re: g_boolean_handled_accumulator patch

[Tim Janik <timj gtk org>]

On Thu, 11 Sep 2003, Owen Taylor wrote:

> Here's a patch to add g_boolean_handled_accumulator().
>
> (http://bugzilla.gnome.org/show_bug.cgi?id=80487)
>
> It adds a testaccumulator.c to testgobject, which includes
> a test for this, and the accumulator testing from
> testgobject.c (I didn't remove it from their, however.)

we're getting a bit too many test files here i think.
can you put this test into a new directory gobject/tests/
and have them executed upon make check please?
(especially since your tests tend to be real checks with
no output).
if you want to take that opportunity to move the other tests
there as well, feel free. i'd just like to keep testgobject.c
as an executable in gobject/ to hack/test random stuff while
implementing/changing things in gobject/.

> I added a separate testmarshalers.[ch] to support the
> test case, since I didn't want to add "random" marshalers
> to gmarshal.h.

that should be in tests/ as well then (unless you mean to explicitely
provide new marhsallers that we'll have to keep offering API-wise through
future versions).

> Index: gobject/gsignal.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gobject/gsignal.c,v
> retrieving revision 1.52
> diff -u -p -u -r1.52 gsignal.c
> --- gobject/gsignal.c	19 Aug 2003 02:15:40 -0000	1.52
> +++ gobject/gsignal.c	11 Sep 2003 19:23:41 -0000
> @@ -2586,6 +2586,22 @@ type_debug_name (GType type)
>      return "<invalid>";
>  }
>
> +gboolean
> +g_boolean_handled_accumulator (GSignalInvocationHint *ihint,
> +			       GValue                *return_accu,
> +			       const GValue          *handler_return,
> +			       gpointer               dummy)
> +{
> +  gboolean continue_emission;
> +  gboolean signal_handled;
> +
> +  signal_handled = g_value_get_boolean (handler_return);
> +  g_value_set_boolean (return_accu, signal_handled);
> +  continue_emission = !signal_handled;
> +
> +  return continue_emission;
> +}
> +
>  /* --- compile standard marshallers --- */
>  #include	"gobject.h"
>  #include	"genums.h"

two things here:
- do we really need this accumulator in glib? afaik, this kind of accumulator
  is used only for events, i.e. widgets, so having gtk provide this seems
  sufficient to me.
- what's the rationale to put the accumulator into the g_boolean_ namespace?
  alternatives are:
  - g_accumulator_ (better if we'll get more accumulators in the future)
  - g_signal_ (only usefull if the accumulator isn't moved to gtk)

> Index: docs/reference/gobject/tmpl/signals.sgml
> ===================================================================
> RCS file: /cvs/gnome/glib/docs/reference/gobject/tmpl/signals.sgml,v
> retrieving revision 1.28
> diff -u -p -u -r1.28 signals.sgml
> --- docs/reference/gobject/tmpl/signals.sgml	18 Apr 2003 00:18:06 -0000	1.28
> +++ docs/reference/gobject/tmpl/signals.sgml	11 Sep 2003 19:23:41 -0000
> @@ -870,3 +870,21 @@ Returns the invocation hint of the inner
>  @Returns:
>
>
> +<!-- ##### FUNCTION g_boolean_handled_accumulator ##### -->
> +<para>
> +A predefined #GSignalAccumulator for signals that return a
> +boolean values. The behavior that this accumulator gives is
> +that a return of %TRUE stops emission of subsequent callbacks,

signals are emitted, callbacks are called or invoked, so
s/emission/invocation/

> +while a return of %FALSE allows emission to coninue. The idea

s/emission/the emission/

> +here is that a %TRUE return indicates that the callback
> +<emphasis>handled</emphasis> the signal, and no further
> +handling is needed.
> +</para>
> +
> + ihint: standard #GSignalAccumulator parameter
> + return_accu: standard #GSignalAccumulator parameter
> + handler_return: standard #GSignalAccumulator parameter
> + dummy: standard #GSignalAccumulator parameter
> + Returns: standard #GSignalAccumulator result

is the word "standard" really necessary (usefull) here?

>
> Look good?
> 				Owen
>

---
ciaoTJ