Re: Minor(ish) changes to PsppireValueEntry

[Ben Pfaff]

John Darrington <address@hidden> writes:

> The attached patch makes some changes to PsppireValueEntry - a few bugs,
> a few cleanups where I thought the code was hard to follow, and a few
> improvements.  Hopefully all self explanitary.
>
> It also uses this widget in the state variable entry of the ROC dialog. 

In general I think that these are good changes.  Thank you.

I think that it would be better to break the PsppireValueEntry
changes into a separate commit.

I have a few detailed comments, inlined below:

> -  if (val_labs_count (obj->val_labs) > 0)
> +  if (obj->val_labs && val_labs_count (obj->val_labs) > 0)

The extra clause above, though harmless, is not necessary because
val_labs_count() returns 0 if passed NULL.

> @@ -269,10 +269,22 @@ psppire_value_entry_refresh_model (PsppireValueEntry 
> *obj)
>      }
>    else
>      model = NULL;
> +  
> +  old_model = gtk_combo_box_get_model (GTK_COMBO_BOX (obj));
> +
> +  if (old_model != model)
> +    {
> +      GtkEntry *entry = GTK_ENTRY (gtk_bin_get_child (GTK_BIN (obj)));
> +      gtk_entry_set_text (entry, "");
> +    }
> +
> +  if (old_model)
> +    g_object_unref (old_model);

gtk_combo_box_get_model() doesn't ref the returned pointer, so
I'm pretty sure this "unref" call is wrong.

>    gtk_combo_box_set_model (GTK_COMBO_BOX (obj), model);
>    gtk_combo_box_entry_set_text_column (GTK_COMBO_BOX_ENTRY (obj), COL_LABEL);
> -  gtk_widget_set_sensitive (entry, model != NULL);
> +  gtk_combo_box_set_button_sensitivity (GTK_COMBO_BOX (obj), model != NULL 
> +                                     ? GTK_SENSITIVITY_ON : 
> GTK_SENSITIVITY_OFF);

I didn't know about gtk_combo_box_set_button_sensitivity().
However, the description of the default "auto" setting seems like
it does the right thing, so I wonder whether we can just remove
the gtk_widget_set_sensitive() call entirely?

The reason for the psppire_value_entry_set_value_labels() change
isn't obvious to me.  Can you explain?

Thanks,

Ben.