On 08/16/2011 05:08 PM, Walter Bender wrote:
On Tue, Aug 16, 2011 at 9:09 AM, Simon Schampijer<[email protected]> wrote:
Hi Walter,
thanks for the patch! Comments inline:
On 08/15/2011 05:16 PM, Walter Bender wrote:
From: Walter Bender<[email protected]>
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 [email protected]
---
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'],
Will take care of this.
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.
The issue is not so much HOME as DOCUMENTS. Let me investigate further.
thanks for the review.
-walter
Ahh, I actually misunderstood a subtle detail all the time: you only
make $HOME/DOCUMENTS available. Indeed for this case it makes sense to
use xdg and not a hardcoded path based on $HOME. The xdg [1] way does
cope with the use case where a user changes the language and as well his
directory names (e.g. s/Documents/Dokumente).
Regards,
Simon
[1] http://www.freedesktop.org/wiki/Software/xdg-user-dirs
_______________________________________________
Sugar-devel mailing list
[email protected]
http://lists.sugarlabs.org/listinfo/sugar-devel
