[Sugar-devel] [PATCH] adding $HOME/Documents to volumestoolbar
simon at schampijer.de
Wed Aug 17 08:10:38 EDT 2011
On 08/16/2011 05:08 PM, Walter Bender wrote:
> On Tue, Aug 16, 2011 at 9:09 AM, Simon Schampijer<simon at schampijer.de> wrote:
>> 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
>>> It also means that Sugar activities can more readily access files
>>> 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
>>> 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.
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  way does
cope with the use case where a user changes the language and as well his
directory names (e.g. s/Documents/Dokumente).
More information about the Sugar-devel