[Sugar-devel] [PATCH] Journal: Fix for sl#1842

[Andrés Ambrois]

On Sunday 13 June 2010 06:21:22 pm anishmangal2002 at gmail.com wrote:
> diff --git a/src/jarabe/journal/journalactivity.py 
b/src/jarabe/journal/journalactivity.py
> index 0559560..9536ca3 100644
> --- a/src/jarabe/journal/journalactivity.py
> +++ b/src/jarabe/journal/journalactivity.py
> @@ -27,6 +27,7 @@ import statvfs
>  import os
>  
>  from sugar.graphics.window import Window
> +from sugar.graphics.alert import NotifyRedAlert
>  from sugar.bundle.bundle import ZipExtractException, RegistrationException
>  from sugar import env
>  from sugar.activity import activityfactory
> @@ -138,6 +139,17 @@ class JournalActivity(Window):
>          self._critical_space_alert = None
>          self._check_available_space()
>  
> +    def _alert_notify_cb(self, _volumes_toolbar, strerror):

This may be useful to show errors coming from other places than VolumesToolbar 
(e.g. ObjectPalette may delete entries and raise errors too), so the second 
parameter may not always be _volumes_toolbar.

> +        alert = NotifyRedAlert(10)
> +        alert.props.title=_('Alert')
> +        alert.props.msg = _(strerror)

Gettexting a variable string will not work unless you're thinking of 
translating all the possible variations, which in any case should already be 
done by the user's system locale. I guess this could also break pootle.

> diff --git a/src/jarabe/journal/volumestoolbar.py 
b/src/jarabe/journal/volumestoolbar.py
> index 74b974c..33e7b88 100644
> --- a/src/jarabe/journal/volumestoolbar.py
> +++ b/src/jarabe/journal/volumestoolbar.py
> @@ -35,6 +35,9 @@ class VolumesToolbar(gtk.Toolbar):
>      __gsignals__ = {
>          'volume-changed': (gobject.SIGNAL_RUN_FIRST,
>                             gobject.TYPE_NONE,
> +                           ([str])),
> +        'data-ioerror': (gobject.SIGNAL_RUN_FIRST,
> +                           gobject.TYPE_NONE,
>                             ([str]))
>      }

This is just a nitpick, but 'data-ioerror' may not be the best name. For 
instance, when we copy an empty without a file to a removable device, 
model.write() throws a ValueError.

> @@ -137,7 +150,11 @@ class BaseButton(RadioToolButton):
>                                 info, timestamp):
>          object_id = selection_data.data
>          metadata = model.get(object_id)
> -        model.copy(metadata, self.mount_point)
> +        try:
> +            model.copy(metadata, self.mount_point)
> +        except IOError as (errno, strerror):
> +            logging.error('BaseButton._drag_data_received_cb: %s; %s' % 
(errno, strerror))
> +            self.emit('data-ioerror', strerror)

If you also catch ValueError here, and with the patch I just sent ("Wrong 
exception when copying an entry with no file to a removable device") you could 
show that error to the user as well.

Nice work!

Reviewed-by: Andrés Ambrois <andresambrois at gmail.com>
-- 
  -Andrés
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://lists.sugarlabs.org/archive/sugar-devel/attachments/20100613/5faedc07/attachment.pgp