[PATCH] Use the new Gtk.ListBox for displaying environments and addons (#1039683).

Chris Lumens clumens at redhat.com
Tue Jan 7 16:23:40 UTC 2014


> > @@ -279,40 +276,58 @@ class SoftwareSelectionSpoke(NormalSpoke):
> >              hubQ.send_message(self.__class__.__name__, _("No installation source available"))
> >              return False
> >  
> > +    def _add_row(self, listbox, name, desc, button):
> I think it would be nicer if the _add_row function instantiated a new
> Gtk.RadioButton on its own. Maybe it could take 'group' instead of
> 'button' as the last parameter and return the newly created button for
> the outer scope to decide on groups?

Well, _add_row is for both environments (you can only select one, thus a
radio button) and addons (you can select multiple, thus a check button).
That's why it takes a button as an argument.

> > +            self._add_row(self._environmentListBox, name, desc, radio)
> > +            if not firstRadio:
> > +                firstRadio = radio
>
> I know you guys don't like those funky one-liners, but I'd prefer
> 
>     firstRadio = firstRadio or radio
> 
> here instead of the if statement. It looks and reads completely natural
> to me.

You're right, but I think it's okay here.  Changed.

> > -        # Then, remove all the groups that were selected by the previously
> > +        button.handler_block_by_func(self.on_button_toggled)
> > +        button.set_active(not button.get_active())
> > +        button.handler_unblock_by_func(self.on_button_toggled)
> Does this (un)blocking really work? I believe I've seen some issues with
> it in other places of our code and the documentation says:
> 
> Blocks all handlers on an instance that match func and data.
> 
> instance :
> 	The instance to block handlers from.
> func :
> 	The C closure callback of the handlers (useless for non-C closures).
> data :
> 	The closure data of the handlers' closures.
> Returns :
> 	The number of handlers that matched.
> 
> If it works, I'm looking forward to start using it and we could file a
> bug on that docs.

Here, it certainly appears to work.  Before adding this code, I was
hitting an infinite loop where calling set_active here was going into
on_button_toggled, which went back into the activate function...

We use it in a very few other places too.

- Chris


More information about the anaconda-patches mailing list