pspp-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Minor(ish) changes to PsppireValueEntry


From: John Darrington
Subject: Re: Minor(ish) changes to PsppireValueEntry
Date: Mon, 23 Apr 2012 10:09:17 +0000
User-agent: Mutt/1.5.18 (2008-05-17)

On Sun, Apr 22, 2012 at 04:37:11PM -0700, Ben Pfaff wrote:
     
     > -  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.

OK.
     
     > @@ -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.

But the model is created with gtk_list_store_new which returns the object with 
a ref count
of 1. Something needs to unref it before the model is replaced with a new one.
     
     >    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?

I'll have a look to see if that does the job.
     
     The reason for the psppire_value_entry_set_value_labels() change
     isn't obvious to me.  Can you explain?
     
I'll rework the changes to make it clearer, and send out another set of patches 
for review.

J'


-- 
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://keys.gnupg.net or any PGP keyserver for public key.

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]