<div class="gmail_quote">On Thu, Oct 14, 2010 at 5:08 PM, Sascha Silbe <span dir="ltr"><<a href="mailto:sascha-ml-reply-to-2010-3@silbe.org">sascha-ml-reply-to-2010-3@silbe.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Excerpts from godiard's message of Thu Oct 14 20:10:14 +0200 2010:<br>
<div class="im"><br>
> Fix the tickets OLPC #5291, OLPC #1925, SL #2127<br>
<br>
</div>Please provide some background information as part of the patch<br>
description. What are you changing and why?<br>
<br>
Please mention the module name as part of the patch summary, e.g.:<br>
<br>
[PATCH Write] keep file type across load/save (SL #2127)<br>
<br></blockquote><div><br>The subject is created by git send-email. I can change the first line from the patch, but no how you say.<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
This<br>
a) ensures maintainers notice patches for their modules and<br>
b) enables reviewers to prioritise patches depending on how well they<br>
know the module.<br>
<div class="im"><br>
<br>
[AbiWordActivity.py]<br>
> @@ -423,28 +423,38 @@ class AbiWordActivity (activity.Activity):<br>
> #self.abiword_canvas.invoke_cmd('com.abisource.abiword.abicollab.olpc.buddyLeft', self.participants[buddy.object_path()], 0, 0)<br>
><br>
> def read_file(self, file_path):<br>
> - logging.debug('AbiWordActivity.read_file: %s, mimetype: %s', file_path, self.metadata['mime_type'])<br>
> - if 'source' in self.metadata and self.metadata['source'] == '1':<br>
> - logger.debug('Opening file in view source mode')<br>
> - self.abiword_canvas.load_file('file://' + file_path, 'text/plain')<br>
> + logging.debug('AbiWordActivity.read_file: %s, mimetype: %s',<br>
> + file_path, self.metadata['mime_type'])<br>
<br>
</div>Please don't mix style changes with bug fixes.<br>
<div class="im"><br></div></blockquote><div><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im">
> + if self.metadata['mime_type'] in ['text/plain', 'text/csv']:<br>
> + logging.debug('Opening file in text mode')<br>
> + self.abiword_canvas.load_file('file://' + file_path, 'text/plain')<br>
> else:<br>
> 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<br>
<br>
</div>I fail to see how this addresses (one of?) the tickets you mentioned.<br>
Also it will cause the activity to break if the Journal entry doesn't<br>
have a MIME type set.<br>
<br></blockquote><div><br>The code correct <a href="http://bugs.sugarlabs.org/ticket/2127">http://bugs.sugarlabs.org/ticket/2127</a> where .csv files are opened and saved like odt files.<br>That is because canvas.load_file must be called with the mime type (and specifically 'text/plain')<br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
[write_file()]<br>
<br>
Same comments as for read_file().<br>
<div class="im"><br>
> + # change the extension in the file name and the description<br>
> + self._change_file_ext(self.metadata['title'], '.odt')<br>
> + self._change_file_ext(self.metadata['description'], '.odt')<br>
<br>
</div>Randomly changing user-set metadata is a no-no.<br>
<br></blockquote><div><br>Not randomly. Write save the files like OpenDocument but don't change the metadata. <br>If you copy the file from the journal to a pen drive or look at the file in the datastore can see it. <br>
It's difficult to see the actual behavior in the patch, but I have tested it a long time.<br><br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Sascha<br>
<font color="#888888"><br>
--<br>
<a href="http://sascha.silbe.org/" target="_blank">http://sascha.silbe.org/</a><br>
<a href="http://www.infra-silbe.de/" target="_blank">http://www.infra-silbe.de/</a><br>
</font><br>_______________________________________________<br>
Sugar-devel mailing list<br>
<a href="mailto:Sugar-devel@lists.sugarlabs.org">Sugar-devel@lists.sugarlabs.org</a><br>
<a href="http://lists.sugarlabs.org/listinfo/sugar-devel" target="_blank">http://lists.sugarlabs.org/listinfo/sugar-devel</a><br>
<br></blockquote></div><br>