[Sugar-devel] [PATCH v2 sugar] Frame: show application-set icon for non-Sugar windows

[Simon Schampijer]

On 09/20/2010 07:32 PM, Sascha Silbe wrote:
> Excerpts from James Cameron's message of Mon Sep 20 02:48:08 +0200 2010:
>
>>> +    def _icon_changed_cb(self, window):
>>> +        gobject.idle_add(self._update_icon, window)
>>
>> I'm puzzled.  Why defer the update to an idle task?  If it is because of
>> a race, I'm worried this won't fix it.
>
> I don't remember why I did it that way originally. Maybe it was from
> when I triggered it (only) from __init__() instead of listening to the
> signal.
> It works fine without the idle task on both XO-1.5 and regular PC, so
> let's just remove it.
>
>>> +    def _update_icon(self, window):
>>> +        logging.debug('_update_icon: start')
>>> +        pixbuf = window.get_icon()
>>> +        if pixbuf is None:
>>> +            return
>>
>> When a function is an idle task, the return value has significance, so
>> I'm not sure if a bare return is the right thing here.
>
> While it does work (bare return is actually return None which gets
> evaluated to False in boolean contexts), it would be bad style.
> But removing the idle task already takes care of that. :)
>
>>> +        self._icon.props.file = util.TempFilePath(path)
>>> +        logging.debug('_update_icon: updated icon')
>>
>> When are these files cleaned up?
>
> sugar.util.TempFilePath is a wrapper around a file name that removes
> the file when the name gets garbage-collected. So the file should
> disappear on the first garbage collection run after the window was
> closed or a new icon assigned.
>
> Sascha

Can we get an updated patch for that one? Looks quite good when 
addressing James comments. (I would remove the loggings).

Regards,
    Simon