[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