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

Vratislav Podzimek vpodzime at redhat.com
Tue Jan 7 16:47:41 UTC 2014


On Tue, 2014-01-07 at 11:23 -0500, Chris Lumens wrote:
> > > @@ -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.
Good then, thanks for double-checking.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list