[Sugar-devel] [PATCH] Change the logic used to determine the format used to save files.

[Sascha Silbe]

Excerpts from godiard's message of Thu Oct 14 20:10:14 +0200 2010:

> Fix the tickets OLPC #5291, OLPC #1925, SL #2127

Please provide some background information as part of the patch
description. What are you changing and why?

Please mention the module name as part of the patch summary, e.g.:

[PATCH Write] keep file type across load/save (SL #2127)

This
a) ensures maintainers notice patches for their modules and
b) enables reviewers to prioritise patches depending on how well they
   know the module.


[AbiWordActivity.py]
> @@ -423,28 +423,38 @@ class AbiWordActivity (activity.Activity):
>          #self.abiword_canvas.invoke_cmd('com.abisource.abiword.abicollab.olpc.buddyLeft', self.participants[buddy.object_path()], 0, 0)
>  
>      def read_file(self, file_path):
> -        logging.debug('AbiWordActivity.read_file: %s, mimetype: %s', file_path, self.metadata['mime_type'])
> -        if 'source' in self.metadata and self.metadata['source'] == '1':
> -            logger.debug('Opening file in view source mode')
> -            self.abiword_canvas.load_file('file://' + file_path, 'text/plain') 
> +        logging.debug('AbiWordActivity.read_file: %s, mimetype: %s',
> +            file_path, self.metadata['mime_type'])

Please don't mix style changes with bug fixes.


> +        if self.metadata['mime_type'] in ['text/plain', 'text/csv']:
> +            logging.debug('Opening file in text mode')
> +            self.abiword_canvas.load_file('file://' + file_path, 'text/plain')
>          else:
>              self.abiword_canvas.load_file('file://' + file_path, '') # we pass no mime/file type, let libabiword autodetect it, so we can handle multiple file formats

I fail to see how this addresses (one of?) the tickets you mentioned.
Also it will cause the activity to break if the Journal entry doesn't
have a MIME type set.


[write_file()]

Same comments as for read_file().

> +                # change the extension in the file name and the description
> +                self._change_file_ext(self.metadata['title'], '.odt')
> +                self._change_file_ext(self.metadata['description'], '.odt')

Randomly changing user-set metadata is a no-no.

Sascha

--
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20101014/0eac2b98/attachment.pgp>