On Sat, Sep 04, 2010 at 05:13:03PM +0200, Sascha Silbe wrote: > Excerpts from Kandarp Kaushik's message of Sat Sep 04 16:16:57 +0200 2010: > > [data/sugar.schemas.in] > > + <locale name="C"> > > + <short>Protected Activities Bundle Indentifiers</short> > > Typo. Also a bit hard to understand. Something along the lines of > "Bundle IDs of protected activities" might be better. > > > + <long>Users will not be allowed to erase these > > + activities through the list view.</long> > > [src/jarabe/desktop/activitieslist.py] > > + if activity_info.is_user_activity() and \ > > + not registry.is_activity_protected(self._bundle_id): > > + menu_item = MenuItem(_('Erase'), 'list-remove') > > + menu_item.connect('activate', self.__erase_activate_cb) > > + self.menu.append(menu_item) > > + menu_item.show() > > + > > + if not os.access(activity_info.get_path(), os.W_OK): > > + menu_item.props.sensitive = False > > This is getting convoluted. Please factor it out and turn your condition > into one or several guards [1]. E.g.: > > def _add_erase_option(self, activity_info): > if not activity_info.is_user_activity(): > return > if registry.is_activity_protected(self._bundle_id): > return > # show menu item etc. > > A suitable docstring is left as an exercise for the reader. ;) > > > [src/jarabe/model/bundleregistry.py] > > + client = gconf.client_get_default() > > + self._protected_activities = > > client.get_list('/desktop/sugar/protected_activities', > > + gconf.VALUE_STRING) > > I have a feeling this will break badly if the GConf value is not set. > See #1147 [2] for a similar case.
I've found that there is no way to call has() method(all sugar code calls get_* methods w/o any checks). If everything is installed, we will get key value in any case (default from shcheme), but if installation is inproper... I've managed to coredump python for unknow keys. > And please don't indent this much for a continuation. Personally I'd use > regular indentation (i.e. four spaces); maybe Tomeu can state his > preference so we can agree on some guideline. > > > PS: Please use git-send-email or some equivalent method of sending > patches. Your message has had at least one line of the patch > wrapped; I would have been unable to try out the patch (because > it wouldn't apply). > > Sascha > > [1] http://en.wikipedia.org/wiki/Guard_(computing) > [2] https://bugs.sugarlabs.org/ticket/1147 > -- > http://sascha.silbe.org/ > http://www.infra-silbe.de/ > _______________________________________________ > Sugar-devel mailing list > Sugar-devel@lists.sugarlabs.org > http://lists.sugarlabs.org/listinfo/sugar-devel -- Aleksey _______________________________________________ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel