Hi Walter,

thanks for the patch! Comments inline:

On 08/15/2011 05:16 PM, Walter Bender wrote:
From: Walter Bender<walter.ben...@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-wal...@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
_______________________________________________
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Reply via email to