Re: [Sugar-devel] Paint: trying to fix #1771

2010-06-11 Thread James Cameron 
On Fri, Jun 11, 2010 at 01:25:06AM -0300, Gonzalo Odiard wrote:
 I think it works, but i don't know if the right thing to do.

 From 97fb2adb1ea97a472e020244ae7a2d22c7a94db3 Mon Sep 17 00:00:00 2001
 From: Gonzalo Odiard godi...@gmail.com
 Date: Fri, 11 Jun 2010 01:22:36 -0300
 Subject: [PATCH] fix #1771 - paint overwrites file type instead of creating 
 new file


Your patch was missing an explanation in the commit message, but during
review I tried to make an explanation.  I'll put the explanation here
and ask you to review it:

Paint only writes type image/png.  It can load other types, but
overwrites as image/png.  This patch detects reading a type that is not
image/png, and forgets the association between the image being edited
and the journal entry that was read, so that the original journal entry
is not overwritten.

Does that match your understanding?

 http://bugs.sugarlabs.org/ticket/1771
 ---
  OficinaActivity.py |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)
 
 diff --git a/OficinaActivity.py b/OficinaActivity.py
 index c72576a..78bc8cf 100644
 --- a/OficinaActivity.py
 +++ b/OficinaActivity.py
 @@ -140,8 +140,8 @@ class OficinaActivity(activity.Activity):
  
  def read_file(self, file_path):
  '''Read file from Sugar Journal.'''
 +print 'reading file', file_path, mime_type, self.metadata
 ['mime_type']
  
 -logging.debug('reading file %s', file_path)

Why did you change this to print?  If it was only temporary, change it
back.  If it is of use, change the logging.debug line.

Hint: since the logging level can't be changed once Sugar is started, I
change debug to error while I am testing code, and then change it
back to debug before committing the patch.

  
  pixbuf = gtk.gdk.pixbuf_new_from_file(file_path)
  
 @@ -155,6 +155,9 @@ class OficinaActivity(activity.Activity):
  self._setup_handle = self.fixed.connect('size_allocate',
  size_allocate_cb)
  
 +if self.metadata['mime_type'] != image/png:
 +self._jobject.object_id = None
 +

A comment would be useful here, because reading the code doesn't explain
why it is happening.  Something like:

# disassociate with journal entry to avoid overwrite (SL #1771)

  def write_file(self, file_path):
  '''Save file on Sugar Journal. '''
  
 --
 1.6.6.1

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Re: [Sugar-devel] Paint: trying to fix #1771

2010-06-11 Thread Gonzalo Odiard 
On Fri, Jun 11, 2010 at 9:04 PM, James Cameron qu...@laptop.org wrote:

 On Fri, Jun 11, 2010 at 01:25:06AM -0300, Gonzalo Odiard wrote:
  I think it works, but i don't know if the right thing to do.
 
  From 97fb2adb1ea97a472e020244ae7a2d22c7a94db3 Mon Sep 17 00:00:00 2001
  From: Gonzalo Odiard godi...@gmail.com
  Date: Fri, 11 Jun 2010 01:22:36 -0300
  Subject: [PATCH] fix #1771 - paint overwrites file type instead of
 creating new file
 

 Your patch was missing an explanation in the commit message, but during
 review I tried to make an explanation.  I'll put the explanation here
 and ask you to review it:

 Paint only writes type image/png.  It can load other types, but
 overwrites as image/png.  This patch detects reading a type that is not
 image/png, and forgets the association between the image being edited
 and the journal entry that was read, so that the original journal entry
 is not overwritten.

 Does that match your understanding?


Yes, it's ok.


  http://bugs.sugarlabs.org/ticket/1771
  ---
   OficinaActivity.py |5 -
   1 files changed, 4 insertions(+), 1 deletions(-)
 
  diff --git a/OficinaActivity.py b/OficinaActivity.py
  index c72576a..78bc8cf 100644
  --- a/OficinaActivity.py
  +++ b/OficinaActivity.py
  @@ -140,8 +140,8 @@ class OficinaActivity(activity.Activity):
 
   def read_file(self, file_path):
   '''Read file from Sugar Journal.'''
  +print 'reading file', file_path, mime_type, self.metadata
  ['mime_type']
 
  -logging.debug('reading file %s', file_path)

 Why did you change this to print?  If it was only temporary, change it
 back.  If it is of use, change the logging.debug line.

 Hint: since the logging level can't be changed once Sugar is started, I
 change debug to error while I am testing code, and then change it
 back to debug before committing the patch.

 Was temporary, but your idea is better for debugging.


 
   pixbuf = gtk.gdk.pixbuf_new_from_file(file_path)
 
  @@ -155,6 +155,9 @@ class OficinaActivity(activity.Activity):
   self._setup_handle = self.fixed.connect('size_allocate',
   size_allocate_cb)
 
  +if self.metadata['mime_type'] != image/png:
  +self._jobject.object_id = None
  +

 A comment would be useful here, because reading the code doesn't explain
 why it is happening.  Something like:

 # disassociate with journal entry to avoid overwrite (SL #1771)

   def write_file(self, file_path):
   '''Save file on Sugar Journal. '''
 
  --
  1.6.6.1

 I was thinking change the title also, because is unclear when you see two
inputs in the journal with the same name but there are two different files.
May be changing the extension or setting the title empty. I don't know what
it's better.



 --
 James Cameron
 http://quozl.linux.org.au/




-- 
Gonzalo Odiard
Responsable de Desarrollo
Sistemas Australes
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

Re: [Sugar-devel] Paint: trying to fix #1771

2010-06-11 Thread James Cameron 
On Fri, Jun 11, 2010 at 10:05:42PM -0300, Gonzalo Odiard wrote:
 I was thinking change the title also, because is unclear when you see
 two inputs in the journal with the same name but there are two
 different files.  May be changing the extension or setting the title
 empty. I don't know what it's better.

The MIME type is not exposed to the user in 0.84, so yes, it is unclear
to the user why they have a new journal entry with the same name, where
one is SVG and the other is PNG.

I don't know what to do about it though, sorry.

-- 
James Cameron
http://quozl.linux.org.au/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

[Sugar-devel] Paint: trying to fix #1771

2010-06-10 Thread Gonzalo Odiard 
I think it works, but i don't know if the right thing to do.

From 97fb2adb1ea97a472e020244ae7a2d22c7a94db3 Mon Sep 17 00:00:00 2001
From: Gonzalo Odiard godi...@gmail.com
Date: Fri, 11 Jun 2010 01:22:36 -0300
Subject: [PATCH] fix #1771 - paint overwrites file type instead of creating
new file

http://bugs.sugarlabs.org/ticket/1771
---
 OficinaActivity.py |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/OficinaActivity.py b/OficinaActivity.py
index c72576a..78bc8cf 100644
--- a/OficinaActivity.py
+++ b/OficinaActivity.py
@@ -140,8 +140,8 @@ class OficinaActivity(activity.Activity):

 def read_file(self, file_path):
 '''Read file from Sugar Journal.'''
+print 'reading file', file_path, mime_type,
self.metadata['mime_type']

-logging.debug('reading file %s', file_path)

 pixbuf = gtk.gdk.pixbuf_new_from_file(file_path)

@@ -155,6 +155,9 @@ class OficinaActivity(activity.Activity):
 self._setup_handle = self.fixed.connect('size_allocate',
 size_allocate_cb)

+if self.metadata['mime_type'] != image/png:
+self._jobject.object_id = None
+
 def write_file(self, file_path):
 '''Save file on Sugar Journal. '''

-- 
1.6.6.1


-- 
Gonzalo Odiard
Responsable de Desarrollo
Sistemas Australes
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel