[Sugar-devel] [PATCH] adding $HOME/Documents to volumestoolbar

[Simon Schampijer]

Hi Walter,

thanks for the patch! Comments inline:

On 08/15/2011 05:16 PM, Walter Bender wrote:
> From: Walter Bender<walter.bender at gmail.com>
>
> This patch adds $HOME/Documents to the volume toolbar in the Journal view.
> The rationale is to make it easier for people to move files in and out of
> the data store from the file system -- a feature oft requested by teachers.
> It also means that Sugar activities can more readily access files generated
> outside of Sugar -- another feature requested by teachers.
>
> This resubmission of the patch includes a suggestion by Silbe that the
> xdg-user-dir call be factored out in a separate (global) function
>
> Note that this patch requires the inclusion of a new icon, user-documents,
> that is included in a separate patch.
>
> See 1313419398-9452-1-git-send-email-walter at sugarlabs.org
>
> ---
>   src/jarabe/journal/volumestoolbar.py |   42 ++++++++++++++++++++++++++++++++++
>   1 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/src/jarabe/journal/volumestoolbar.py b/src/jarabe/journal/volumestoolbar.py
> index 72b5918..e1d1721 100644
> --- a/src/jarabe/journal/volumestoolbar.py
> +++ b/src/jarabe/journal/volumestoolbar.py
> @@ -16,6 +16,8 @@
>
>   import logging
>   import os
> +import subprocess
> +import errno
>   import statvfs
>   from gettext import gettext as _
>
> @@ -53,6 +55,19 @@ def _get_id(document):
>           return None
>

The _get_documents_path() has the wrong indentation.

> +def _get_documents_path():

Please add a docstring here.

> +        try:
> +            pipe = subprocess.Popen(['xdg-user-dir', 'DOCUMENTS'],

You are introducing a new dependency here: xdg-user-dirs. Any specific 
reason why you do not want to use the HOME env variable?

 From a quick survey: I guess relying on the env variable HOME like you 
do in your other patch would be fine as well. You could as well do 
"os.getenv('XDG_DATA_HOME', os.path.expanduser('~/'))" so once a 
xdg_data_home is set we do not fail (we do this as well in the toolkit), 
but I would maybe fix this up in another go if we want to.

> +                                    stdout=subprocess.PIPE)
> +            documents_path = pipe.communicate()[0].strip()
> +            if os.path.exists(documents_path):
> +                return documents_path
> +        except OSError, exception:
> +            if exception.errno != errno.ENOENT:
> +                logging.exception('Could not run xdg-user-dir')
> +        return None
> +
> +
>   def _convert_entries(root):
>       """Convert entries written by the datastore version 0.
>
> @@ -189,6 +204,8 @@ class VolumesToolbar(gtk.Toolbar):
>           volume_monitor.disconnect(self._mount_removed_hid)
>
>       def _set_up_volumes(self):
> +        self._set_up_documents_button()
> +
>           volume_monitor = gio.volume_monitor_get()
>           self._mount_added_hid = volume_monitor.connect('mount-added',
>                                                          self.__mount_added_cb)
> @@ -198,6 +215,19 @@ class VolumesToolbar(gtk.Toolbar):
>           for mount in volume_monitor.get_mounts():
>               self._add_button(mount)
>
> +    def _set_up_documents_button(self):
> +        documents_path = _get_documents_path()
> +        if documents_path is not None:
> +            button = DocumentsButton(documents_path)
> +            button.props.group = self._volume_buttons[0]
> +            button.set_palette(Palette(_('Documents')))
> +            button.connect('toggled', self._button_toggled_cb)
> +            button.show()
> +
> +            self.insert(button, -1)
> +            self._volume_buttons.append(button)
> +            self.show()
> +
>       def __mount_added_cb(self, volume_monitor, mount):
>           self._add_button(mount)
>
> @@ -372,3 +402,15 @@ class JournalButtonPalette(Palette):
>           self._progress_bar.props.fraction = fraction
>           self._free_space_label.props.label = _('%(free_space)d MB Free') % \
>                   {'free_space': free_space / (1024 * 1024)}
> +
> +
> +class DocumentsButton(BaseButton):
> +
> +    def __init__(self, documents_path):
> +        BaseButton.__init__(self, mount_point=documents_path)
> +
> +        self.props.named_icon = 'user-documents'
> +
> +        client = gconf.client_get_default()
> +        color = XoColor(client.get_string('/desktop/sugar/user/color'))
> +        self.props.xo_color = color

Regards,
    Simon