Ooh...  And here's what gnumeric does.  No memory allocation, at least, but
too much work entirely.

static const char *search_type_group[] = {
	"search_type_text",
	"search_type_regexp",
	NULL
};
...
	i = gnumeric_glade_group_value (gui, search_type_group);


int
gnumeric_glade_group_value (GladeXML *gui, const char *group[])
{
	int i;
	for (i = 0; group[i]; i++) {
		GtkWidget *w = glade_xml_get_widget (gui, group[i]);
		if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (w)))
			return i;
	}
	return -1;
}

<owen> federico: But a get_active() that returns an index is also going to have a
       "what's the order' problem
<owen> federico: I think you'd have to make the index a property ... nothing else
       is sufficiently stable

Yeah, perhaps

enum {
  RADIO_FOO,
  RADIO_BAR,
  RADIO_BAZ
};

gtk_radio_button_set_id (myradio1, RADIO_FOO);
gtk_radio_button_set_id (myradio2, RADIO_BAR);
gtk_radio_button_set_id (myradio3, RADIO_BAZ);

...

int id = gtk_radio_button_get_active_id (radio);
if (id == RADIO_FOO)
    ....

And if the id is not set, _get_active_id() could simply return the index of the
active button, relative to the order in which buttons were inserted in the group.

We should really make a good job of publicizing this, pointing to a nice section
in the Migration docs.

Compare how we solved this for GtkRadioAction, where each action has an integer
"value", and gtk_radio_action_get_current_value() returns the value of the
active group member.

Regardless of method, without glade/libglade support this is close to pointless.

Yes, we should have the same terminology as for GtkRadioAction:  a "value"
property, and a get_current_value method.

If the "value" property is not set for any of the items in the group, it should
probably return_if_fail or something to tell the programmer that he forgot to
set some values.

This fell through the cracks.  Let's take care of it for 2.10.

we can try again for 2.12...

Created attachment 85662 [details] [review]
initial implementation

let's try for gtk+ 2.12. this patch implements a value property in GtkRadioButton and its accessors.

to mimic gtk_radio_action_get_current_value() we should have a gtk_radio_button_group_get_current_value(GSList *group) which is a bit clunky and hard to bind in other languages with native list types; hence, I preferred an approach where the radio button is queried for its value. this should map to something like:

void
radio_button_toggled_cb (GtkToggleButton *toggle_button, gpointer data)
{
  GtkRadioButton *radio_button = GTK_RADIO_BUTTON (toggle_button);
  
  if (!gtk_toggle_button_get_active (toggle_button))
    return;

  active = gtk_radio_button_get_value (radio_button);
  switch (active)
    {
    case RADIO_OPTION_FOO:
      foo ()
      break;
    case RADIO_OPTION_BAR:
      bar ();
      break;
    case RADIO_OPTION_BAZ:
      baz ();
      break;
    }
}

where the RADIO_OPTION_* ids are defined into an enum and set for each radio button using gtk_radio_button_set_value().

Comment on attachment 85662 [details] [review]
initial implementation

W00t!  Ebassi takes the hero medal!

>+static GQuark radio_button_value_quark = 0;

I don't really like using GObject data to attach random bits.  A private structure would be better.

>+  g_object_class_install_property (gobject_class,
>+                                   PROP_VALUE,
>+                                   g_param_spec_int ("value",
>+                                                     P_("Value"),
>+                                                     P_("The value bound to the radio button"),
>+                                                     G_MININT, G_MAXINT - 1,
>+                                                     G_MAXINT - 1,
>+                                                     GTK_PARAM_READWRITE));
>+

You have
GTK_RADIO_BUTTON_VALUE_UNSET = G_MAXINT - 1

Then I guess you want to make the maximum value G_MAXINT - 2, since values are clamped to the specified limits inclusive.  Or just make GTK_RADIO_BUTTON_VALUE_UNSET = G_MAXINT and leave the limits as they are.

>+  if (retval == GTK_RADIO_BUTTON_VALUE_UNSET)
>+    retval = g_slist_index (radio_button->group, radio_button);

Remember that new radio items get prepended to the list, so you want to subtract the g_slist_index() from the number of elements in the list.

[Your test program could have caught that, but it doesn't exercise untagged radio items :) ]

This is a lovely patch, Ebassi :)  Thanks for taking care of it!

crap, totally forgot about this one - and bugzilla didn't send me a mail with your comment federico. will work on this ASAP and attach a patch - maybe we can still shovel this API in before the freeze.

Created attachment 89011 [details] [review]
[PATCH] Add value property to GtkRadioButton


This patch adds a GtkRadioButton:value property which can be set to an
arbitrary integer value. This value can then be checked into the toggled
signal handlers in order to know which button in a group has been toggled.

If the value is not set (using gtk_radio_button_set_value()), then
gtk_radio_button_get_value() will return the position of the radio button
inside the group.
---
 gtk/gtkradiobutton.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++-
 gtk/gtkradiobutton.h |    4 +-
 tests/testgtk.c      |   31 +++++++++++++
 3 files changed, 149 insertions(+), 3 deletions(-)

While using get_value is sufficient for a toggled handler, I still think it would be good to have a get_current_value() to determine the current value for a radio group from the outside (ie when not in a toggled handler).

I believe something like 

gint
gtk_radio_button_get_current_value (GtkRadioButton *button)
{
  GSList *list;

  for (list = button->group; list; list = list->next)
    {
      GtkRadioButton *r = list->data;

      if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (r))
        return gtk_radio_button_get_value (r);
    }
  
  return GTK_RADIO_BUTTON_VALUE_UNSET;
}

should work and is not bad for language bindings.

Other than that, the patch looks fine to me.

Created attachment 89288 [details] [review]
[PATCH] Add function for getting the current value


This incremental patch adds gtk_radio_button_get_current_value() to
the GtkRadioButton API. This function returns the value for the currently
active GtkRadioButton inside a radio buttons group.

This patch also adds the new API to the exported symbols and to the
documentation build.
---
 docs/reference/gtk/gtk-sections.txt |    3 +++
 gtk/gtk.symbols                     |    3 +++
 gtk/gtkradiobutton.c                |   31 +++++++++++++++++++++++++++++++
 gtk/gtkradiobutton.h                |    1 +
 4 files changed, 38 insertions(+), 0 deletions(-)

Looks good; but it needs to be "Since: 2.12" for GTK+...

committed with the docs fix to trunk.

2007-06-04  Emmanuele Bassi  <ebassi@gnome.org>

        * gtk/gtkradiobutton.h:
        * gtk/gtkradiobutton.c:
        * gtk/gtk.symbols: Add gtk_radio_button_set_value(),
        gtk_radio_button_get_current_value() and gtk_radio_button_get_value().
        Use these functions to set and get an arbitrary integer associated to
        a GtkRadioButton in a group, like the value associated to a
        GtkRadioAction.

        * tests/testgtk.c:
        (create_radio_buttons), (radio_toggled_cb): Exercise the new API.

Comment on attachment 89288 [details] [review]
[PATCH] Add function for getting the current value

reopening because i'm fairly unhappy with the patch that went into SVN:

>diff --git a/gtk/gtkradiobutton.c b/gtk/gtkradiobutton.c
>index d2a68a0..c458aaf 100644
>--- a/gtk/gtkradiobutton.c
>+++ b/gtk/gtkradiobutton.c
>@@ -855,5 +855,36 @@ gtk_radio_button_set_value (GtkRadioButton *radio_button,
>     }
> }
> 
>+/**
>+ * gtk_radio_button_get_current_value:
>+ * @radio_button: a #GtkRadioButton
>+ *
>+ * Returns the value of the currently selected radio button inside
>+ * the same group of @radio_button.
>+ *
>+ * Return value: the value set with gtk_radio_button_set_value() or
>+ *   the current position inside the group.
>+ *
>+ * Since: 2.14
>+ */

"_value" is a particularly unspecific name (and makes me think of our Adjustment API fwiw). something like gtk_radio_button_group_index() would be a much better name if we just provided a getter. in case we really have to have a setter for this as well that allows arbitrary customized IDs, the API could use names like gtk_radio_button_{set|get}_group_index_id(). though i still don't see the necessesity for providing a setter here. anyone care to elaborate the reason/use case/problem for that?

>+struct _GtkRadioButtonPrivate
>+{
>+  gint value;
>+};

adding an extra int here is really overkill. unless we expect this integer to be used for >90% of all the radio buttons out there, we shouldn't waste an extra integer for *every* radio button instance that is created for gtk+ versions >= 2.12. i.e. we could save it by disallowing '0' as index_id (or aliasing that to the automatically counted group_index), and then use GObject QData.

>+  if (priv->value != value)
>+    {
>+      g_object_ref (radio_button);
>+
>+      priv->value = value;
>+
>+      g_object_notify (G_OBJECT (radio_button), "value");
>+      g_object_unref (radio_button);

there is no need for the ref/unref guards here. g_object_notify does it's own ref/unref, and priva->value=; can't possibly call user code so doesn't need to be reentrant.

>+    }
>+}

(In reply to comment #16)
> (From update of attachment 89288 [details] [review] [edit])
> reopening because i'm fairly unhappy with the patch that went into SVN:
> 

> >+/**
> >+ * gtk_radio_button_get_current_value:
> >+ * @radio_button: a #GtkRadioButton
> >+ *
> >+ * Returns the value of the currently selected radio button inside
> >+ * the same group of @radio_button.
> >+ *

> "_value" is a particularly unspecific name (and makes me think of our
> Adjustment API fwiw). something like gtk_radio_button_group_index() would be a
> much better name if we just provided a getter. in case we really have to have a
> setter for this as well that allows arbitrary customized IDs, the API could use
> names like gtk_radio_button_{set|get}_group_index_id().

if we favour consistency with GtkRadioAction then we should use value here as well, or change it there. we can avoid the setter and just use a construct-only property, with a new constructor pair:

  gtk_radio_button_new_with_value (GSList *group);
  gtk_radio_button_new_with_value_from_widget (GtkWidget *widget);

since the id will most likely be the same for the entire lifetime of the widget.

> though i still don't
> see the necessesity for providing a setter here. anyone care to elaborate the
> reason/use case/problem for that?

I didn't think about a construct-only property, but the ability to set an arbitrary id is obviously needed if I change the radio buttons group; if I only use the position in the list and then remove a widget later on, all my checks in the toggled callbacks will break.

> >+struct _GtkRadioButtonPrivate
> >+{
> >+  gint value;
> >+};
> 
> adding an extra int here is really overkill. unless we expect this integer to
> be used for >90% of all the radio buttons out there, we shouldn't waste an
> extra integer for *every* radio button instance that is created for gtk+
> versions >= 2.12. i.e. we could save it by disallowing '0' as index_id (or
> aliasing that to the automatically counted group_index), and then use GObject
> QData.

I don't have strong preferences for either approach..

> >+  if (priv->value != value)
> >+    {
> >+      g_object_ref (radio_button);
> >+
> >+      priv->value = value;
> >+
> >+      g_object_notify (G_OBJECT (radio_button), "value");
> >+      g_object_unref (radio_button);
> 
> there is no need for the ref/unref guards here. g_object_notify does it's own
> ref/unref, and priva->value=; can't possibly call user code so doesn't need to
> be reentrant.

will remove.

Created attachment 89335 [details] [review]
[PATCH] Add an arbitrary value to a GtkRadioButton


New patch, using g_object_qdata() instead of creating a private structure
for a single integer. This patch also flags GtkRadioButton:value as a
G_PARAM_CONSTRUCT_ONLY property, since the value should be the same for the
entire lifetime of the widget. The setter has been removed and two new
constructors have been added: gtk_radio_button_new_with_value() and
gtk_radio_button_new_with_value_from_widget().

I have no strong feeling in making the GtkRadioButton:value property modifiable
after construction and adding the setter back, though.
---
 gtk/gtk.symbols      |    4 +
 gtk/gtkradiobutton.c |  178 +++++++++++++++++++++++++++++++++++++++++++++++++-
 gtk/gtkradiobutton.h |    7 ++-
 tests/testgtk.c      |   42 ++++++++++--
 4 files changed, 219 insertions(+), 12 deletions(-)

Comment on attachment 89335 [details] [review]
[PATCH] Add an arbitrary value to a GtkRadioButton

> This patch also flags GtkRadioButton:value as a
> G_PARAM_CONSTRUCT_ONLY property, since the value should be the same for the
> entire lifetime of the widget.

in general, i tend to tell people to avoid construct properties where possible, because they always make the API-users life harder. e.g. a programmer might need to get at a value earlier than it's convenient for him, or builder programs need to jump through extra hoops (recreating widgets) when all they want is to just change a property value. so for the sake of the (API) user, i'd recommend to keep this as a normal modifiable property unless you have strong compelling reasons for not doing so.

>diff --git a/gtk/gtkradiobutton.c b/gtk/gtkradiobutton.c
>index b4de838..acbd36f 100644
>--- a/gtk/gtkradiobutton.c
>+++ b/gtk/gtkradiobutton.c
>@@ -32,13 +32,16 @@
> #include "gtkintl.h"
> #include "gtkalias.h"
> 
>+#define GTK_RADIO_BUTTON_VALUE_UNSET    (0)

i think having a macro for this is overkill, using plain 0 will be clearer as i elaborate later on.

>+
>+typedef struct _GtkRadioButtonPrivate   GtkRadioButtonPrivate;
> 
> enum {
>   PROP_0,
>-  PROP_GROUP
>+  PROP_GROUP,
>+  PROP_VALUE
> };
> 
>-
> static void     gtk_radio_button_destroy        (GtkObject           *object);
> static gboolean gtk_radio_button_focus          (GtkWidget           *widget,
> 						 GtkDirectionType     direction);
>@@ -53,10 +56,13 @@ static void     gtk_radio_button_get_property   (GObject             *object,
> 						 guint                prop_id,
> 						 GValue              *value,
> 						 GParamSpec          *pspec);
>+static void     gtk_radio_button_set_value      (GtkRadioButton      *radio_button,
>+                                                 gint                 value);
> 
> G_DEFINE_TYPE (GtkRadioButton, gtk_radio_button, GTK_TYPE_CHECK_BUTTON)
> 
> static guint group_changed_signal = 0;
>+static GQuark quark_value = 0;
> 
> static void
> gtk_radio_button_class_init (GtkRadioButtonClass *class)
>@@ -73,6 +79,8 @@ gtk_radio_button_class_init (GtkRadioButtonClass *class)
>   button_class = (GtkButtonClass*) class;
>   check_button_class = (GtkCheckButtonClass*) class;
> 
>+  quark_value = g_quark_from_static_string ("gtk-radio-button-value");
>+
>   gobject_class->set_property = gtk_radio_button_set_property;
>   gobject_class->get_property = gtk_radio_button_get_property;
> 
>@@ -83,6 +91,25 @@ gtk_radio_button_class_init (GtkRadioButtonClass *class)
> 							P_("The radio button whose group this widget belongs to."),
> 							GTK_TYPE_RADIO_BUTTON,
> 							GTK_PARAM_WRITABLE));
>+  /**
>+   * GtkRadioButton:value:
>+   *
>+   * The value is an arbitrary integer which can be used as a
>+   * convenient way to determine which button in a group is
>+   * currently active in a ::toggled signal handler.
>+   *
>+   * Since: 2.12
>+   */
>+  g_object_class_install_property (gobject_class,
>+                                   PROP_VALUE,
>+                                   g_param_spec_int ("value",
>+                                                     P_("Value"),
>+                                                     P_("The value bound to the radio button"),
>+                                                     G_MININT, G_MAXINT - 1,
>+                                                     GTK_RADIO_BUTTON_VALUE_UNSET,
>+                                                     G_PARAM_CONSTRUCT_ONLY | GTK_PARAM_READWRITE));

hm, if the value is meant as an integer id, i personally would have defaulted to using guint instead of int, since i see litle point in negative ids.

>+
>+  
>   object_class->destroy = gtk_radio_button_destroy;
> 
>   widget_class->focus = gtk_radio_button_focus;
>@@ -129,6 +156,10 @@ gtk_radio_button_init (GtkRadioButton *radio_button)
> 
>   _gtk_button_set_depressed (GTK_BUTTON (radio_button), TRUE);
>   gtk_widget_set_state (GTK_WIDGET (radio_button), GTK_STATE_ACTIVE);
>+
>+  g_object_set_qdata (G_OBJECT (radio_button),
>+                      quark_value,
>+                      GINT_TO_POINTER (GTK_RADIO_BUTTON_VALUE_UNSET));

using s/GTK_RADIO_BUTTON_VALUE_UNSET/0/ here, helps to see that this call is uneccessary.
any QData value that is queried and was nevert set will be returned as NULL.
so, for all newly created instances, this call just wastes CPU cycles and should be removed.

> }
> 
> static void
>@@ -147,7 +178,7 @@ gtk_radio_button_set_property (GObject      *object,
>       GtkRadioButton *button;
> 
>     case PROP_GROUP:
>-        button = g_value_get_object (value);
>+      button = g_value_get_object (value);
> 
>       if (button)
> 	slist = gtk_radio_button_get_group (button);
>@@ -155,6 +186,9 @@ gtk_radio_button_set_property (GObject      *object,
> 	slist = NULL;
>       gtk_radio_button_set_group (radio_button, slist);
>       break;
>+    case PROP_VALUE:
>+      gtk_radio_button_set_value (radio_button, g_value_get_int (value));
>+      break;
>     default:
>       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>       break;
>@@ -167,8 +201,13 @@ gtk_radio_button_get_property (GObject    *object,
> 			       GValue     *value,
> 			       GParamSpec *pspec)
> {
>+  GtkRadioButton *radio_button = GTK_RADIO_BUTTON (object);
>+
>   switch (prop_id)
>     {
>+    case PROP_VALUE:
>+      g_value_set_int (value, gtk_radio_button_get_value (radio_button));
>+      break;
>     default:
>       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
>       break;
>@@ -341,6 +380,62 @@ gtk_radio_button_new_with_mnemonic_from_widget (GtkRadioButton *group,
>   return gtk_radio_button_new_with_mnemonic (l, label);
> }
> 
>+/**
>+ * gtk_radio_button_new_with_value:
>+ * @group: the radio button group
>+ * @value: an arbitrary integer
>+ *
>+ * Creates a new #GtkRadioButton adding it to @group. Sets @value
>+ * as the value that should be returned by gtk_radio_button_get_value()
>+ * and gtk_radio_button_get_current_value().
>+ *
>+ * Return value: a new #GtkRadioButton
>+ *
>+ * Since: 2.12
>+ */
>+GtkWidget *
>+gtk_radio_button_new_with_value (GSList *group,
>+                                 gint    value)
>+{
>+  GtkWidget *radio_button;
>+
>+  radio_button = g_object_new (GTK_TYPE_RADIO_BUTTON,
>+                               "value", value,
>+                               NULL);
>+
>+  if (group)
>+    gtk_radio_button_set_group (GTK_RADIO_BUTTON (radio_button), group);
>+
>+  return radio_button;
>+}
>+
>+/**
>+ * gtk_radio_button_new_with_value_from_widget:
>+ * @group: widget to get radio group from or %NULL
>+ * @value: an arbitrary integer
>+ *
>+ * Creates a new #GtkRadioButton and adds it to the same group as
>+ * @group. Sets @value as the value that should be returned by
>+ * gtk_radio_button_get_value() and gtk_radio_button_get_current_value().
>+ *
>+ * Return value: a new #GtkRadioButton
>+ *
>+ * Since: 2.12
>+ */
>+GtkWidget *
>+gtk_radio_button_new_with_value_from_widget (GtkRadioButton *group,
>+                                             gint            value)
>+{
>+  GSList *l = NULL;
>+
>+  g_return_val_if_fail (group == NULL || GTK_IS_RADIO_BUTTON (group), NULL);
>+
>+  if (group)
>+    l = gtk_radio_button_get_group (group);
>+
>+  return gtk_radio_button_new_with_value (l, value);
>+}
>+
> GSList*
> gtk_radio_button_get_group (GtkRadioButton *radio_button)
> {
>@@ -742,5 +837,82 @@ gtk_radio_button_draw_indicator (GtkCheckButton *check_button,
>     }
> }
> 
>+/**
>+ * gtk_radio_button_get_value:
>+ * @radio_button: a #GtkRadioButton
>+ *
>+ * Retrieve the value property of the @radio_button set using
>+ * gtk_radio_button_new_with_value(). If no value was set, the position
>+ * of @radio_button in the radio button group is returned.
>+ *
>+ * Return value: the value property of the radio button
>+ *
>+ * Since: 2.12
>+ */
>+gint
>+gtk_radio_button_get_value (GtkRadioButton *radio_button)
>+{
>+  gint retval;
>+
>+  g_return_val_if_fail (GTK_IS_RADIO_BUTTON (radio_button), 0);
>+
>+  retval = GPOINTER_TO_INT (g_object_get_qdata (G_OBJECT (radio_button),
>+                                                quark_value));
>+
>+  if (retval == GTK_RADIO_BUTTON_VALUE_UNSET)

here as well, '0' would be clearer to use as "unset" value, because 0/NULL is generally known to be the "unset" value for QData already. also, the docs should clearly mention that '0' is used as unset value, so the user avoids using/setting 0 and expecting value to be not-unset.

>+    {
>+      gint index = g_slist_index (radio_button->group, radio_button);
>+      gint length = g_slist_length (radio_button->group);
>+      
>+      retval = length - index;
>+    }
>+
>+  return retval;
>+}
>+
>+static void
>+gtk_radio_button_set_value (GtkRadioButton *radio_button,
>+                            gint            value)
>+{
>+  g_return_if_fail (GTK_IS_RADIO_BUTTON (radio_button));
>+
>+  g_object_set_qdata (G_OBJECT (radio_button),
>+                      quark_value,
>+                      GINT_TO_POINTER (value));
>+
>+  g_object_notify (G_OBJECT (radio_button), "value");
>+}
>+
>+/**
>+ * gtk_radio_button_get_current_value:
>+ * @radio_button: a #GtkRadioButton
>+ *
>+ * Returns the value of the currently selected radio button inside
>+ * the same group of @radio_button.
>+ *
>+ * Return value: the value set with gtk_radio_button_set_value() or
>+ *   the current position inside the group.
>+ *
>+ * Since: 2.12
>+ */
>+gint
>+gtk_radio_button_get_current_value (GtkRadioButton *radio_button)

i think having gtk_radio_button_get_value *and* gtk_radio_button_get_current_value in the API is fairly confusing.
while the former is a straight accessor, the current function is a bit of a misnomer, because the function really operates on the radio button group. so i'd say gtk_radio_button_group_get_active_value or similar would be clearer, and less confusing with get_value.

>+{
>+  GSList *l;
>+
>+  g_return_val_if_fail (GTK_IS_RADIO_BUTTON (radio_button),
>+                        GTK_RADIO_BUTTON_VALUE_UNSET);
>+
>+  for (l = radio_button->group; l; l = l->next)
>+    {
>+      GtkToggleButton *button = GTK_TOGGLE_BUTTON (l->data);
>+
>+      if (gtk_toggle_button_get_active (button))
>+        return gtk_radio_button_get_value (GTK_RADIO_BUTTON (l->data));
>+    }
>+
>+  return GTK_RADIO_BUTTON_VALUE_UNSET;
>+}
>+
> #define __GTK_RADIO_BUTTON_C__
> #include "gtkaliasdef.c"

>diff --git a/tests/testgtk.c b/tests/testgtk.c
>index 44eb4fc..b8502b4 100644
>--- a/tests/testgtk.c
>+++ b/tests/testgtk.c

hm, can i ask you to please rework the test case to end up in gtk-demo? ;)
gtk-demo is what people should really look at for code examples, and your new API would be very nice to have a demo for that people can copy and paste.

(In reply to comment #19)
> in general, i tend to tell people to avoid construct properties where possible,
> because they always make the API-users life harder. e.g. a programmer might
> need to get at a value earlier than it's convenient for him, or builder
> programs need to jump through extra hoops (recreating widgets) when all they
> want is to just change a property value. so for the sake of the (API) user, i'd
> recommend to keep this as a normal modifiable property unless you have strong
> compelling reasons for not doing so.

okay, so I'll just add the accessors and remove the constructors new_with_value() and new_with_value_from_widget(), and make the property read-write.

> >+gint
> >+gtk_radio_button_get_current_value (GtkRadioButton *radio_button)
> 
> i think having gtk_radio_button_get_value *and*
> gtk_radio_button_get_current_value in the API is fairly confusing.
> while the former is a straight accessor, the current function is a bit of a
> misnomer, because the function really operates on the radio button group. so
> i'd say gtk_radio_button_group_get_active_value or similar would be clearer,
> and less confusing with get_value.

binding gtk_radio_button_group_get_active_value() will be a pain. bindings for languages with a native list type would still need to expose that function as GtkRadioButton::get_active_value(), so we'd gained nothing.

anyway, I can make the changes and will attach a patch asap.

What about ::get_choosen_value()?

*** Bug 451636 has been marked as a duplicate of this bug. ***

Created attachment 90793 [details] [review]
adds two functions in order to retrieve the first active radio button in a group

This proposed patch adds two functions :

- gtk_radio_button_get_active that will retrieve the first active (if any) radio button within a group taken as an argument,
- gtk_radio_button_get_active_from_widget which will do the same job but with
the group of the radio button argument.

*** Bug 322819 has been marked as a duplicate of this bug. ***

This would be great. At the moment I've only used groups in gtkmm, where I just made a vector<pair<int, RadioButton>> and match on that to get/set the active ID... but it seemed like a lot of unnecessary faff, even despite being a lot more concise than the C equivalent.

*** Bug 728712 has been marked as a duplicate of this bug. ***

Review of attachment 90793 [details] [review]:

::: ../gtk+-2.11.3/gtk/gtkradiobutton.c
@@ +354,3 @@
+ * @group: the radio button group
+ * 
+ * @returns: the active #GtkRadioButton within the group @group

It's "Returns:"

@@ +356,3 @@
+ * @returns: the active #GtkRadioButton within the group @group
+ **/
+GtkWidget* 

There's a missing space between the type name and asterisk.

@@ +357,3 @@
+ **/
+GtkWidget* 
+gtk_radio_button_get_active (GSList *group)

The title is about getting the index. How does getting the widget help towards that aim? We already know we can hack the index by getting the widget and finding it in the list of children, but that's verbose and brittle. This may be a step along the way, but I'm not sure it is useful enough on its own to put in.

@@ +384,3 @@
+  if (radio_group_member)
+    return gtk_radio_button_get_active (radio_group_member->group);
+}

You let radio_group_member be NULL but don't document it, and worse, you don't return anything and hence invoke nasal demons if it *is* NULL... If NULLs are allowed, whether in or out, that fact must be documented, along with what the NULL means - and the code must be safe in both the non-NULL and NULL cases.

@@ +82,3 @@
 void       gtk_radio_button_set_group                     (GtkRadioButton *radio_button,
                                                            GSList         *group);
+GtkWidget* gtk_radio_button_get_active                    (GSList         *group);

The asterisk should be right-aligned to the function name (even if other, older functions already get this wrong)

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/243.