Bug 11020 - Synchronize with Google Reader.
: Synchronize with Google Reader.
Status: RESOLVED FIXED
Product: ReSiStance
General
: unspecified
: All Maemo
: Unspecified enhancement (vote)
: ---
Assigned To: Sergio Villar Senin
: general
:
:
:
:
  Show dependency tree
 
Reported: 2010-07-28 19:35 UTC by Chus
Modified: 2011-01-12 17:30 UTC (History)
2 users (show)

See Also:


Attachments
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (8.74 KB, patch)
2010-10-31 15:15 UTC, Chus
Details
A patch to mark as read and as not read (from ReSiStance to Google Reader) (4.33 KB, patch)
2010-10-31 21:22 UTC, Chus
Details
A patch to synchronize a subscription (you can mark as sinchronized) and it is going to appear in Google Reader. Something similar when you delete a sinchronized feed. (5.93 KB, patch)
2010-11-01 15:20 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (11.11 KB, patch)
2010-11-05 18:31 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (11.94 KB, patch)
2010-11-08 21:23 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (14.07 KB, patch)
2010-11-17 01:09 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (17.16 KB, patch)
2010-11-19 22:26 UTC, Chus
Details
A patch to mark as read and as not read (from ReSiStance to Google Reader) (5.29 KB, patch)
2010-11-22 20:46 UTC, Chus
Details
A patch to mark as read and as not read (from ReSiStance to Google Reader) (5.44 KB, patch)
2010-12-04 14:36 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (preparing release 0.7) (18.02 KB, patch)
2010-12-04 19:34 UTC, Chus
Details
A patch to mark as read and as not read (from ReSiStance to Google Reader) (preparing release 0.7) (5.33 KB, patch)
2010-12-04 20:41 UTC, Chus
Details
A patch to synchronize a subscription (you can mark as sinchronized) and it is going to appear in Google Reader. Something similar when you delete a sinchronized feed. (preparing release 0.7) (6.36 KB, patch)
2010-12-04 22:59 UTC, Chus
Details
A patch to synchronize with Google Reader. It allows you to add the feeds that are in Google Reader. (preparing release 0.7) (18.31 KB, patch)
2010-12-05 14:06 UTC, Chus
Details
A patch to synchronize automatically items with google. (8.55 KB, patch)
2010-12-06 05:08 UTC, Chus
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description Chus (reporter) 2010-07-28 19:35:56 UTC
Now, Google Reader is a very important web-based aggregator. It could be
interesting synchronize ReSiStance with it.
Comment 1 Sergio Villar Senin 2010-08-09 10:45:41 UTC
Yes, yes, yes and yes

That's an absolutely killer feature I'd love to have
Comment 2 Chus (reporter) 2010-10-31 15:15:23 UTC
Created an attachment (id=3151) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader.
Comment 3 Chus (reporter) 2010-10-31 21:22:22 UTC
Created an attachment (id=3152) [details]
A patch to mark as read and as not read (from ReSiStance to Google Reader)
Comment 4 Chus (reporter) 2010-11-01 15:20:20 UTC
Created an attachment (id=3154) [details]
A patch to synchronize a subscription (you can mark as sinchronized) and it is
going to appear in Google Reader. Something similar when you delete a
sinchronized feed.
Comment 5 Sergio Villar Senin 2010-11-02 11:37:43 UTC
(From update of attachment 3151 [details])
><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">diff --git a/src/ReSiStance/feedmanager.py b/src/ReSiStance/feedmanager.py
>index 3fa1131..6f1b2b7 100755
>--- a/src/ReSiStance/feedmanager.py
>+++ b/src/ReSiStance/feedmanager.py
>@@ -25,6 +25,7 @@ import gobject
> import constants
> import cPickle
> import urllib
>+import urllib2
> import urlparse
> import os
> import base64
>@@ -94,11 +95,11 @@ class FeedManager(gobject.GObject):
>     def get_feed_list(self):
>         return self.feed_data_list
> 
>-    def add_feed(self, url, callback, data=None):
>+    def add_feed(self, url, callback, data=None, sync=False):

callback and data are normally the last 2 parameters. So I'd move the sync one
to the third place

> 
>         # Create a new thread to update from network
>         adding_thread = Thread(target=self._add_feed_in_thread,
>-                               args=(url, callback, data))
>+                               args=(url, callback, data, sync))
>         adding_thread.start()
> 
>     def update_feed(self, feed, callback, data=None):
>@@ -221,6 +222,47 @@ class FeedManager(gobject.GObject):
>             callback(key_words, dialog, infolist)
>             gtk.gdk.threads_leave()
> 

I'd change the "synchronize" name for something more descriptive. As it's
specific code for google reader I think we can rename it to
sync_with_google_reader for example

>+    def synchronize(self, user, password, callback, data=None):
>+        # Create a new thread to search
>+        synchronize_thread = Thread(target=self._synchronize_in_thread,
>+                               args=(user, password, callback, data))
>+
>+        return synchronize_thread.start()
>+
>+    def _synchronize_in_thread(self, user, password, callback, user_data):
>+        params = urllib.urlencode({"service": "reader", "Email": user, "Passwd": password})
>+        url = "https://www.google.com/accounts/ClientLogin"
>+        try:
>+            content = urllib2.urlopen(url,params).read()
>+        except:
>+            gtk.gdk.threads_enter()
>+            callback(None, None, authentication=False)
>+            gtk.gdk.threads_leave()
>+            return 
>+        
>+        pos_begin = content.find('Auth=')
>+        pos_end = content.find('\n', pos_begin)
>+        auth = content[pos_begin+len('Auth='):pos_end]
>+        
>+        try:
>+            opener = urllib2.build_opener()
>+            opener.addheaders = [('Authorization', 'GoogleLogin auth='+auth)]
>+            token = opener.open('http://www.google.com/reader/api/0/token').read()
>+            req = urllib2.Request('http://www.google.com/reader/api/0/subscription/list?output=xml&client=-')
>+            req.add_header('Authorization', 'GoogleLogin auth='+auth)
>+            r = urllib2.urlopen(req)
>+            
>+            doc = minidom.parse(r)
>+            nodes = doc.getElementsByTagName('object')[1:]
>+            for node in nodes:
>+                element = node.getElementsByTagName("string")
>+                feed = element[0].firstChild.data
>+                self._add_feed_in_thread(feed[len('feed/'):], callback, user_data, True)
>+        except: 
>+            gtk.gdk.threads_enter()
>+            callback(None, None)
>+            gtk.gdk.threads_leave()
>+            
>     def save(self, callback, data=None):
>         # TODO: migrate to "with" statement when available
>         try:
>@@ -302,7 +344,7 @@ class FeedManager(gobject.GObject):
> 
>         return pixbuf
> 
>-    def _add_feed_in_thread(self, url, callback, user_data):
>+    def _add_feed_in_thread(self, url, callback, user_data, sync):

Again, move the sync parameter to the third place

> 
>         parsed_url = urlparse.urlsplit(url)
>         if parsed_url.scheme == '':
>@@ -319,6 +361,10 @@ class FeedManager(gobject.GObject):
>             return
> 
>         new_feed_data = ReSiStanceFeedDict(feedparser.parse(url))
>+        
>+        if not 'sync' in new_feed_data:
>+            new_feed_data['sync'] = sync 
>+        

This shouldn't be here. I uploaded some of changes a couple of weeks ago. Every
new field must be initialized in ResistanceFeedDict _init_ function

>         self.feed_data_list.append(new_feed_data)
> 
>         if 'link' in new_feed_data.feed:
>diff --git a/src/ReSiStance/gui.py b/src/ReSiStance/gui.py
>index 15ed8c4..4c536e0 100755
>--- a/src/ReSiStance/gui.py
>+++ b/src/ReSiStance/gui.py
>@@ -201,6 +201,23 @@ class SettingsDialog(gtk.Dialog):
>             font_size_index = constants.font_size_range.index(16)
>         self.selector_font_size.set_active(0, font_size_index)
> 
>+        self.entryUser = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        self.entryUser.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL)
>+
>+        caption = gtk.HBox(False, 16)
>+        caption.pack_start(gtk.Label(_('User') + ':'), False, False)
>+        caption.pack_start(self.entryUser)
>+        self.vbox.add(caption)
>+
>+        self.entryPassword = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        self.entryPassword.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL)

For passwords you should also set HILDON_GTK_INPUT_MODE_INVISIBLE in order to
display asterisks instead of the actual characters of the password. (not sure
if you need also to call gtk_entry_set_visibility tough)

>+
>+        caption = gtk.HBox(False, 16)
>+        caption.pack_start(gtk.Label(_('Password') + ':'), False, False)
>+        caption.pack_start(self.entryPassword)
>+        self.vbox.add(caption)
>+
>+
> class NewFeedDialog(gtk.Dialog):
> 
>     def __init__(self, parent):
>@@ -257,6 +274,8 @@ class FeedsWindow(hildon.StackableWindow):
>     def __init__(self):
>         super(FeedsWindow, self).__init__()
> 
>+        
>+

Remove these extra lines

>         # Feeds
>         self.view = FeedsView(gtk.HILDON_UI_MODE_NORMAL)
>         self.view.set_model (gtk.ListStore(gtk.gdk.Pixbuf, str, gobject.TYPE_PYOBJECT, str))
>@@ -402,6 +421,11 @@ class FeedsWindow(hildon.StackableWindow):
>         button.connect('clicked', self._find_feed_cb)
>         menu.append(button)
> 
>+        button = hildon.GtkButton(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        button.set_label(_('Synchronize'))
>+        button.connect('clicked', self._synchronize_feed_cb)
>+        menu.append(button)
>+

Again I'm not sure about the "sychronize" label. Could you please check how do
other google reader clients name those buttons?

>         menu.show_all()
>         self.set_app_menu(menu)
> 
>@@ -410,6 +434,17 @@ class FeedsWindow(hildon.StackableWindow):
>         dialog.connect('response', self._new_feed_response_cb)
>         dialog.show_all()
> 
>+    def _synchronize_feed_cb(self, button):
>+        stack = hildon.WindowStack.get_default()
>+        window = stack.peek()
>+
>+        if settings.user=='' or settings.password=='':
>+            dialog = SettingsDialog(self)
>+            dialog.connect('response', self._preferences_response_cb, True)
>+            dialog.show_all()
>+        else:
>+            manager.synchronize(settings.user, settings.password, self._feed_added_cb)
>+
>     def _remove_feed_cb(self, button):
> 
>         # tree view edit mode
>@@ -511,8 +546,13 @@ class FeedsWindow(hildon.StackableWindow):
> 
>             dialog.destroy()
> 
>-    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None):
>+    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None, authentication=True):
> 
>+        if authentication == False:
>+            message = _('Could not authenticate. Review the settings data.')
>+            hildon.hildon_banner_show_information(self, '', message)
>+            return
>+        

In case it's not difficult, instead of just showing a banner I'd launch the
settings dialog with the focus set to the username entry.

>         feed_data_list = manager.get_feed_list()
>         if len(feed_data_list) > 0:
>             self.export_opml_button.show()
>@@ -546,13 +586,17 @@ class FeedsWindow(hildon.StackableWindow):
>         hildon.hildon_gtk_window_take_screenshot(self, False)
>         hildon.hildon_gtk_window_take_screenshot(self, True)
> 
>-    def _preferences_response_cb(self, dialog, response):
>+    def _preferences_response_cb(self, dialog, response, sync=False):
> 
>         if response == gtk.RESPONSE_ACCEPT:
>             settings.auto_load_images = dialog.auto_load_images.get_active()
>             settings.rotation_mode = dialog.selector_orientation.get_active(0)
>             settings.default_font_size = int(dialog.selector_font_size.get_current_text())
>+            settings.user = dialog.entryUser.get_text()
>+            settings.password = dialog.entryPassword.get_text()
>             dialog.destroy()
>+            if sync:
>+                manager.synchronize(settings.user, settings.password, self._feed_added_cb)
> 
>             # Update rotation
>             self.rotation_object.set_mode(settings.rotation_mode)
>diff --git a/src/ReSiStance/settings.py b/src/ReSiStance/settings.py
>index a127b54..b32f10a 100644
>--- a/src/ReSiStance/settings.py
>+++ b/src/ReSiStance/settings.py
>@@ -40,6 +40,8 @@ class Settings(object):
>         self.default_font_size = 16
>         self.auto_load_images = True
>         self.rotation_mode = FremantleRotation.AUTOMATIC
>+        self.user  = ''
>+        self.password = ''
> 
>     def load(self):
>         if not (os.path.exists(constants.RSS_CONF_FILE) and
>@@ -56,6 +58,8 @@ class Settings(object):
>         self.default_font_size = self.config['default_font_size']
>         self.auto_load_images = self.config['auto_load_images']
>         self.rotation_mode = self.config['rotation_mode']

Take into account that for older versions of ReSiStance those keys (user and
password) do not exists, and thus the program will fail. You have to check if
they exist before setting them. I guess you'd have to set it to "" if they do
not exist.

>+        self.user = self.config['user']
>+        self.password = self.config['password']
> 
>     def save(self):
>         if self.config == None:
>@@ -66,5 +70,7 @@ class Settings(object):
>         self.config['default_font_size'] = self.default_font_size
>         self.config['auto_load_images'] = self.auto_load_images
>         self.config['rotation_mode'] = self.rotation_mode
>+        self.config['user'] = self.user
>+        self.config['password'] = self.password
> 
>         self.config.write()
Comment 6 Sergio Villar Senin 2010-11-02 12:04:30 UTC
BTW I have just realized that you show the settings dialog if the user/password
are empty, do it also when the authentication fails.

And something I forgot to comment. In the settings window add a label before
the entry/password entries with something like "Synchronize with Google Reader"
in order to separate it from the other settings. And regarding the
user/password label and entries they must be aligned.
Comment 7 Chus (reporter) 2010-11-05 18:31:13 UTC
Created an attachment (id=3160) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader.

About the 'synchronize' label, I saw that other applications use 'synchronize
feeds'. Then, you can choose Google Reader or something like it.
Comment 8 Sergio Villar Senin 2010-11-07 14:50:58 UTC
(From update of attachment 3160 [details])
Patch is getting better. I'm really sorry for not spotting these before but I
still have a couple of comments.

><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">diff --git a/src/ReSiStance/feedmanager.py b/src/ReSiStance/feedmanager.py
>index 18dc884..a56a59b 100755
>--- a/src/ReSiStance/feedmanager.py
>+++ b/src/ReSiStance/feedmanager.py
>@@ -25,6 +25,7 @@ import gobject
> import constants
> import cPickle
> import urllib
>+import urllib2
> import urlparse
> import os
> import base64
>@@ -75,7 +76,7 @@ class LinkParser(SGMLParser):
> 
> class ReSiStanceFeedDict(feedparser.FeedParserDict):
> 
>-    def __init__(self, feed_data):
>+    def __init__(self, feed_data, sync=None):
>         super(ReSiStanceFeedDict, self).__init__()
> 
>         self.update(feed_data)
>@@ -88,6 +89,10 @@ class ReSiStanceFeedDict(feedparser.FeedParserDict):
>             if not 'read' in entry:
>                 entry['read'] = False
> 
>+        if sync != None:
>+            if not 'sync' in self:
>+                self['sync'] = sync
>+
> class FeedManager(gobject.GObject):
> 
>     def __init__(self):
>@@ -98,13 +103,14 @@ class FeedManager(gobject.GObject):
>     def get_feed_list(self):
>         return self.feed_data_list
> 
>-    def add_feed(self, url, callback, data=None):
>+    def add_feed(self, url, sync, callback, data=None):
> 
>         # Create a new thread to update from network
>         adding_thread = Thread(target=self._add_feed_in_thread,
>-                               args=(url, callback, data))
>+                               args=(url, sync, callback, data))
>         adding_thread.start()
> 
>+
>     def update_feed(self, feed, callback, data=None):
> 
>         # Create a new thread to update from network
>@@ -195,7 +201,7 @@ class FeedManager(gobject.GObject):
>                 if node.getAttribute('xmlUrl') != '':
>                     url = node.attributes['xmlUrl'].value
>                     # Add feed to manager
>-                    self._add_feed_in_thread(url, callback, user_data)
>+                    self._add_feed_in_thread(url, False, callback, user_data)
>         except IOError :
>             gtk.gdk.threads_enter()
>             callback(None,None)
>@@ -225,6 +231,47 @@ class FeedManager(gobject.GObject):
>             callback(key_words, dialog, infolist)
>             gtk.gdk.threads_leave()
> 
>+    def sync_with_google_reader(self, user, password, callback, data=None):
>+        # Create a new thread to search
>+        sync_with_google_reader_thread = Thread(target=self._sync_with_google_reader_in_thread,
>+                               args=(user, password, callback, data))
>+
>+        return sync_with_google_reader_thread.start()
>+
>+    def _sync_with_google_reader_in_thread(self, user, password, callback, user_data):
>+        params = urllib.urlencode({"service": "reader", "Email": user, "Passwd": password})
>+        url = "https://www.google.com/accounts/ClientLogin"

Add all these google URL's as constants in constants.py file

>+        try:
>+            content = urllib2.urlopen(url,params).read()
>+        except:
>+            gtk.gdk.threads_enter()
>+            callback(None, None, authentication=False)
>+            gtk.gdk.threads_leave()
>+            return 
>+        
>+        pos_begin = content.find('Auth=')
>+        pos_end = content.find('\n', pos_begin)
>+        auth = content[pos_begin+len('Auth='):pos_end]
>+        
>+        try:
>+            opener = urllib2.build_opener()
>+            opener.addheaders = [('Authorization', 'GoogleLogin auth='+auth)]
>+            token = opener.open('http://www.google.com/reader/api/0/token').read()
>+            req = urllib2.Request('http://www.google.com/reader/api/0/subscription/list?output=xml&client=-')
>+            req.add_header('Authorization', 'GoogleLogin auth='+auth)
>+            r = urllib2.urlopen(req)

Same here, all those google URL's must be constants

>+            
>+            doc = minidom.parse(r)
>+            nodes = doc.getElementsByTagName('object')[1:]
>+            for node in nodes:
>+                element = node.getElementsByTagName("string")
>+                feed = element[0].firstChild.data
>+                self._add_feed_in_thread(feed[len('feed/'):], True, callback, user_data)
>+        except: 
>+            gtk.gdk.threads_enter()
>+            callback(None, None)
>+            gtk.gdk.threads_leave()
>+
>     def save(self, callback, data=None):
>         # TODO: migrate to "with" statement when available
>         try:
>@@ -313,7 +360,7 @@ class FeedManager(gobject.GObject):
> 
>         return pixbuf
> 
>-    def _add_feed_in_thread(self, url, callback, user_data):
>+    def _add_feed_in_thread(self, url, sync, callback, user_data):
> 
>         parsed_url = urlparse.urlsplit(url)
>         if parsed_url.scheme == '':
>@@ -329,7 +376,8 @@ class FeedManager(gobject.GObject):
>             gtk.gdk.threads_leave()
>             return
> 
>-        new_feed_data = ReSiStanceFeedDict(feedparser.parse(url))
>+        new_feed_data = ReSiStanceFeedDict(feedparser.parse(url), sync)
>+        
>         self.feed_data_list.append(new_feed_data)
> 
>         if 'link' in new_feed_data.feed:
>diff --git a/src/ReSiStance/gui.py b/src/ReSiStance/gui.py
>index 28eb338..ec741ff 100755
>--- a/src/ReSiStance/gui.py
>+++ b/src/ReSiStance/gui.py
>@@ -176,6 +176,35 @@ class SettingsDialog(gtk.Dialog):
>             # defaults to 16pt
>             font_size_index = constants.font_size_range.index(16)
>         self.selector_font_size.set_active(0, font_size_index)
>+        
>+        label = gtk.Label(_('Synchronize with Google Reader'))
>+        self.vbox.add(label)
>+        
>+        self.entryUser = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        self.entryUser.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL)
>+
>+        self.entryPassword = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        self.entryPassword.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL | gtk.HILDON_GTK_INPUT_MODE_INVISIBLE)
>+        
>+        cap1 = gtk.VBox(False, 16)
>+        capH1 = gtk.HBox(False, 16)
>+        capH2 = gtk.HBox(False, 16)
>+        labelUser = gtk.Label(_('User') + ':')
>+        labelPassword = gtk.Label(_('Password') + ':')
>+        capH1.pack_end(labelUser, False, False)
>+        capH2.pack_end(labelPassword, False, False)
>+        cap1.add(capH1)
>+        cap1.add(capH2)
>+        
>+        cap2 = gtk.VBox(True, 16)
>+        cap2.pack_start(self.entryUser)
>+        cap2.pack_start(self.entryPassword)
>+        
>+        caption = gtk.HBox(False, 16)
>+        caption.pack_start(cap1, False, False)
>+        caption.pack_start(cap2, False, False)
>+        
>+        self.vbox.add(caption)

A couple of issues with this dialog. 
   * It's better to create 2 hboxes, 1 for the username label and entry and
another one for the password.
   * Entries must be expanded till right border of the dialog.
   * Align labels to the left and do not include semicolons
   * When I test it, the label "Synchornize with google reader" is too short. I
think you have to set it finger height or something, because the letters are
not completely shown

> class NewFeedDialog(gtk.Dialog):
> 
>@@ -427,6 +456,11 @@ class FeedsWindow(hildon.StackableWindow):
>         button.connect('clicked', self._find_feed_cb)
>         menu.append(button)
> 
>+        button = hildon.GtkButton(gtk.HILDON_SIZE_FINGER_HEIGHT)
>+        button.set_label(_('Synchronize'))
>+        button.connect('clicked', self._synchronize_feed_cb)
>+        menu.append(button)
>+
>         menu.show_all()
>         self.set_app_menu(menu)
> 
>@@ -435,6 +469,18 @@ class FeedsWindow(hildon.StackableWindow):
>         dialog.connect('response', self._new_feed_response_cb)
>         dialog.show_all()
> 
>+    def _synchronize_feed_cb(self, button):
>+        stack = hildon.WindowStack.get_default()
>+        window = stack.peek()
>+
>+        if settings.user=='' or settings.password=='':
>+            dialog = SettingsDialog(self)
>+            dialog.connect('response', self._preferences_response_cb, True)
>+            dialog.show_all()
>+        else:
>+            manager.sync_with_google_reader(settings.user, settings.password, self._feed_added_cb)
>+
>+
>     def _remove_feed_cb(self, button):
> 
>         # tree view edit mode
>@@ -532,11 +578,20 @@ class FeedsWindow(hildon.StackableWindow):
>             row_reference = gtk.TreeRowReference(self.view.get_model(), path)
> 
>             # Add feed to manager
>-            manager.add_feed(url, self._feed_added_cb, row_reference)
>+            manager.add_feed(url, False, self._feed_added_cb, row_reference)
> 
>             dialog.destroy()
> 
>-    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None):
>+    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None, authentication=True):
>+
>+        if authentication == False:
>+            message = _('Could not authenticate. Review the settings data.')
>+            hildon.hildon_banner_show_information(self, '', message)
>+            
>+            dialog = SettingsDialog(self)
>+            dialog.connect('response', self._preferences_response_cb, True)
>+            dialog.show_all()
>+            return
> 
>         feed_data_list = manager.get_feed_list()
>         if len(feed_data_list) > 0:
>@@ -573,13 +628,17 @@ class FeedsWindow(hildon.StackableWindow):
>         hildon.hildon_gtk_window_take_screenshot(self, False)
>         hildon.hildon_gtk_window_take_screenshot(self, True)
> 
>-    def _preferences_response_cb(self, dialog, response):
>+    def _preferences_response_cb(self, dialog, response, sync=False):
> 
>         if response == gtk.RESPONSE_ACCEPT:
>             settings.auto_load_images = dialog.auto_load_images.get_active()
>             settings.rotation_mode = dialog.selector_orientation.get_active(0)
>             settings.default_font_size = int(dialog.selector_font_size.get_current_text())
>+            settings.user = dialog.entryUser.get_text()
>+            settings.password = dialog.entryPassword.get_text()
>             dialog.destroy()
>+            if sync:
>+                manager.sync_with_google_reader(settings.user, settings.password, self._feed_added_cb)
> 
>             # Update rotation
>             self.rotation_object.set_mode(settings.rotation_mode)
>@@ -672,7 +731,7 @@ class FeedsWindow(hildon.StackableWindow):
>             row_reference = gtk.TreeRowReference(self.view.get_model(), path)
> 
>             # Add feed to manager
>-            manager.add_feed(url, self._feed_added_cb, row_reference)
>+            manager.add_feed(url, False, self._feed_added_cb, row_reference)
> 
>     def _found_cb(self, key_words, dialog, feeds_info):
>         if feeds_info == None:
>diff --git a/src/ReSiStance/settings.py b/src/ReSiStance/settings.py
>index a127b54..8d4548b 100644
>--- a/src/ReSiStance/settings.py
>+++ b/src/ReSiStance/settings.py
>@@ -40,6 +40,8 @@ class Settings(object):
>         self.default_font_size = 16
>         self.auto_load_images = True
>         self.rotation_mode = FremantleRotation.AUTOMATIC
>+        self.user  = ''
>+        self.password = ''
> 
>     def load(self):
>         if not (os.path.exists(constants.RSS_CONF_FILE) and
>@@ -56,7 +58,15 @@ class Settings(object):
>         self.default_font_size = self.config['default_font_size']
>         self.auto_load_images = self.config['auto_load_images']
>         self.rotation_mode = self.config['rotation_mode']
>-
>+        if 'user' in self.config:
>+            self.user = self.config['user']
>+        else:
>+            self.config['user'] = ''
>+        if 'password' in self.config:
>+            self.password = self.config['password']
>+        else:
>+            self.config['password'] = ''

Use better the compact syntax added in python2.5

self.user = self.config['user'] if 'user' in self.config else ''
self.password = self.config['password'] if 'password' in self.config else ''

>     def save(self):
>         if self.config == None:
>             self.config = ConfigObj(constants.RSS_CONF_FILE, configspec=self.spec)
>@@ -66,5 +76,7 @@ class Settings(object):
>         self.config['default_font_size'] = self.default_font_size
>         self.config['auto_load_images'] = self.auto_load_images
>         self.config['rotation_mode'] = self.rotation_mode
>+        self.config['user'] = self.user
>+        self.config['password'] = self.password
> 
>         self.config.write()
Comment 9 Sergio Villar Senin 2010-11-07 14:54:34 UTC
BTW I have been thinking about it and IMHO it's better not to add a label with
"Synchronize with google reader" and better use a check button. If that button
is checked then the user and password entries will be available, otherwise they
must be dimmed  (I'm thinking about using gtk_widget_set_sensitive)
Comment 10 Sergio Villar Senin 2010-11-07 15:07:04 UTC
Another issue I have just discovered. I think the _preferences_response_cb is
not correctly implemented. It has the sync default to False, this means that if
you open the preferences dialog and add a user and a password it won't
syncrhonize anything. What we should do in this case is pick the value from the
new check button that will decide whether to synchronize or not and use that in
that callback. Then you won't need to pass the sync as user_data. parametter.
Obviously that sync boolean should be saved in the settings the same as the
user and the password. That way user can add user and password and decide later
not to synchronize

Also, you're not filling the user and password entries when the preferences
dialog is opened again.
Comment 11 Chus (reporter) 2010-11-08 00:23:33 UTC
(In reply to comment #8)
> (From update of attachment 3160 [details] [details])
> Patch is getting better. I'm really sorry for not spotting these before but I
> still have a couple of comments.
> 
> ><html><body><pre style="word-wrap: break-word; white-space: pre-wrap;">diff --git a/src/ReSiStance/feedmanager.py b/src/ReSiStance/feedmanager.py
> >index 18dc884..a56a59b 100755
> >--- a/src/ReSiStance/feedmanager.py
> >+++ b/src/ReSiStance/feedmanager.py
> >@@ -25,6 +25,7 @@ import gobject
> > import constants
> > import cPickle
> > import urllib
> >+import urllib2
> > import urlparse
> > import os
> > import base64
> >@@ -75,7 +76,7 @@ class LinkParser(SGMLParser):
> > 
> > class ReSiStanceFeedDict(feedparser.FeedParserDict):
> > 
> >-    def __init__(self, feed_data):
> >+    def __init__(self, feed_data, sync=None):
> >         super(ReSiStanceFeedDict, self).__init__()
> > 
> >         self.update(feed_data)
> >@@ -88,6 +89,10 @@ class ReSiStanceFeedDict(feedparser.FeedParserDict):
> >             if not 'read' in entry:
> >                 entry['read'] = False
> > 
> >+        if sync != None:
> >+            if not 'sync' in self:
> >+                self['sync'] = sync
> >+
> > class FeedManager(gobject.GObject):
> > 
> >     def __init__(self):
> >@@ -98,13 +103,14 @@ class FeedManager(gobject.GObject):
> >     def get_feed_list(self):
> >         return self.feed_data_list
> > 
> >-    def add_feed(self, url, callback, data=None):
> >+    def add_feed(self, url, sync, callback, data=None):
> > 
> >         # Create a new thread to update from network
> >         adding_thread = Thread(target=self._add_feed_in_thread,
> >-                               args=(url, callback, data))
> >+                               args=(url, sync, callback, data))
> >         adding_thread.start()
> > 
> >+
> >     def update_feed(self, feed, callback, data=None):
> > 
> >         # Create a new thread to update from network
> >@@ -195,7 +201,7 @@ class FeedManager(gobject.GObject):
> >                 if node.getAttribute('xmlUrl') != '':
> >                     url = node.attributes['xmlUrl'].value
> >                     # Add feed to manager
> >-                    self._add_feed_in_thread(url, callback, user_data)
> >+                    self._add_feed_in_thread(url, False, callback, user_data)
> >         except IOError :
> >             gtk.gdk.threads_enter()
> >             callback(None,None)
> >@@ -225,6 +231,47 @@ class FeedManager(gobject.GObject):
> >             callback(key_words, dialog, infolist)
> >             gtk.gdk.threads_leave()
> > 
> >+    def sync_with_google_reader(self, user, password, callback, data=None):
> >+        # Create a new thread to search
> >+        sync_with_google_reader_thread = Thread(target=self._sync_with_google_reader_in_thread,
> >+                               args=(user, password, callback, data))
> >+
> >+        return sync_with_google_reader_thread.start()
> >+
> >+    def _sync_with_google_reader_in_thread(self, user, password, callback, user_data):
> >+        params = urllib.urlencode({"service": "reader", "Email": user, "Passwd": password})
> >+        url = "https://www.google.com/accounts/ClientLogin"
> 
> Add all these google URL's as constants in constants.py file
> 
> >+        try:
> >+            content = urllib2.urlopen(url,params).read()
> >+        except:
> >+            gtk.gdk.threads_enter()
> >+            callback(None, None, authentication=False)
> >+            gtk.gdk.threads_leave()
> >+            return 
> >+        
> >+        pos_begin = content.find('Auth=')
> >+        pos_end = content.find('\n', pos_begin)
> >+        auth = content[pos_begin+len('Auth='):pos_end]
> >+        
> >+        try:
> >+            opener = urllib2.build_opener()
> >+            opener.addheaders = [('Authorization', 'GoogleLogin auth='+auth)]
> >+            token = opener.open('http://www.google.com/reader/api/0/token').read()
> >+            req = urllib2.Request('http://www.google.com/reader/api/0/subscription/list?output=xml&client=-')
> >+            req.add_header('Authorization', 'GoogleLogin auth='+auth)
> >+            r = urllib2.urlopen(req)
> 
> Same here, all those google URL's must be constants
> 
> >+            
> >+            doc = minidom.parse(r)
> >+            nodes = doc.getElementsByTagName('object')[1:]
> >+            for node in nodes:
> >+                element = node.getElementsByTagName("string")
> >+                feed = element[0].firstChild.data
> >+                self._add_feed_in_thread(feed[len('feed/'):], True, callback, user_data)
> >+        except: 
> >+            gtk.gdk.threads_enter()
> >+            callback(None, None)
> >+            gtk.gdk.threads_leave()
> >+
> >     def save(self, callback, data=None):
> >         # TODO: migrate to "with" statement when available
> >         try:
> >@@ -313,7 +360,7 @@ class FeedManager(gobject.GObject):
> > 
> >         return pixbuf
> > 
> >-    def _add_feed_in_thread(self, url, callback, user_data):
> >+    def _add_feed_in_thread(self, url, sync, callback, user_data):
> > 
> >         parsed_url = urlparse.urlsplit(url)
> >         if parsed_url.scheme == '':
> >@@ -329,7 +376,8 @@ class FeedManager(gobject.GObject):
> >             gtk.gdk.threads_leave()
> >             return
> > 
> >-        new_feed_data = ReSiStanceFeedDict(feedparser.parse(url))
> >+        new_feed_data = ReSiStanceFeedDict(feedparser.parse(url), sync)
> >+        
> >         self.feed_data_list.append(new_feed_data)
> > 
> >         if 'link' in new_feed_data.feed:
> >diff --git a/src/ReSiStance/gui.py b/src/ReSiStance/gui.py
> >index 28eb338..ec741ff 100755
> >--- a/src/ReSiStance/gui.py
> >+++ b/src/ReSiStance/gui.py
> >@@ -176,6 +176,35 @@ class SettingsDialog(gtk.Dialog):
> >             # defaults to 16pt
> >             font_size_index = constants.font_size_range.index(16)
> >         self.selector_font_size.set_active(0, font_size_index)
> >+        
> >+        label = gtk.Label(_('Synchronize with Google Reader'))
> >+        self.vbox.add(label)
> >+        
> >+        self.entryUser = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
> >+        self.entryUser.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL)
> >+
> >+        self.entryPassword = hildon.Entry(gtk.HILDON_SIZE_FINGER_HEIGHT)
> >+        self.entryPassword.set_input_mode(gtk.HILDON_GTK_INPUT_MODE_FULL | gtk.HILDON_GTK_INPUT_MODE_INVISIBLE)
> >+        
> >+        cap1 = gtk.VBox(False, 16)
> >+        capH1 = gtk.HBox(False, 16)
> >+        capH2 = gtk.HBox(False, 16)
> >+        labelUser = gtk.Label(_('User') + ':')
> >+        labelPassword = gtk.Label(_('Password') + ':')
> >+        capH1.pack_end(labelUser, False, False)
> >+        capH2.pack_end(labelPassword, False, False)
> >+        cap1.add(capH1)
> >+        cap1.add(capH2)
> >+        
> >+        cap2 = gtk.VBox(True, 16)
> >+        cap2.pack_start(self.entryUser)
> >+        cap2.pack_start(self.entryPassword)
> >+        
> >+        caption = gtk.HBox(False, 16)
> >+        caption.pack_start(cap1, False, False)
> >+        caption.pack_start(cap2, False, False)
> >+        
> >+        self.vbox.add(caption)
> 
> A couple of issues with this dialog. 
>    * It's better to create 2 hboxes, 1 for the username label and entry and
> another one for the password.
>    * Entries must be expanded till right border of the dialog.
>    * Align labels to the left and do not include semicolons
>    * When I test it, the label "Synchornize with google reader" is too short. I
> think you have to set it finger height or something, because the letters are
> not completely shown
> 

I think that to align entries, I have to set a width. But, how can I do this? I
find no suitable value (HILDON_SIZE_AUTO_WIDTH, HILDON_SIZE_HALFSCREEN_WIDTH,
HILDON_SIZE_FULLSCREEN_WIDTH).

> > class NewFeedDialog(gtk.Dialog):
> > 
> >@@ -427,6 +456,11 @@ class FeedsWindow(hildon.StackableWindow):
> >         button.connect('clicked', self._find_feed_cb)
> >         menu.append(button)
> > 
> >+        button = hildon.GtkButton(gtk.HILDON_SIZE_FINGER_HEIGHT)
> >+        button.set_label(_('Synchronize'))
> >+        button.connect('clicked', self._synchronize_feed_cb)
> >+        menu.append(button)
> >+
> >         menu.show_all()
> >         self.set_app_menu(menu)
> > 
> >@@ -435,6 +469,18 @@ class FeedsWindow(hildon.StackableWindow):
> >         dialog.connect('response', self._new_feed_response_cb)
> >         dialog.show_all()
> > 
> >+    def _synchronize_feed_cb(self, button):
> >+        stack = hildon.WindowStack.get_default()
> >+        window = stack.peek()
> >+
> >+        if settings.user=='' or settings.password=='':
> >+            dialog = SettingsDialog(self)
> >+            dialog.connect('response', self._preferences_response_cb, True)
> >+            dialog.show_all()
> >+        else:
> >+            manager.sync_with_google_reader(settings.user, settings.password, self._feed_added_cb)
> >+
> >+
> >     def _remove_feed_cb(self, button):
> > 
> >         # tree view edit mode
> >@@ -532,11 +578,20 @@ class FeedsWindow(hildon.StackableWindow):
> >             row_reference = gtk.TreeRowReference(self.view.get_model(), path)
> > 
> >             # Add feed to manager
> >-            manager.add_feed(url, self._feed_added_cb, row_reference)
> >+            manager.add_feed(url, False, self._feed_added_cb, row_reference)
> > 
> >             dialog.destroy()
> > 
> >-    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None):
> >+    def _feed_added_cb(self, pixbuf, new_feed_data, row_reference=None, authentication=True):
> >+
> >+        if authentication == False:
> >+            message = _('Could not authenticate. Review the settings data.')
> >+            hildon.hildon_banner_show_information(self, '', message)
> >+            
> >+            dialog = SettingsDialog(self)
> >+            dialog.connect('response', self._preferences_response_cb, True)
> >+            dialog.show_all()
> >+            return
> > 
> >         feed_data_list = manager.get_feed_list()
> >         if len(feed_data_list) > 0:
> >@@ -573,13 +628,17 @@ class FeedsWindow(hildon.StackableWindow):
> >         hildon.hildon_gtk_window_take_screenshot(self, False)
> >         hildon.hildon_gtk_window_take_screenshot(self, True)
> > 
> >-    def _preferences_response_cb(self, dialog, response):
> >+    def _preferences_response_cb(self, dialog, response, sync=False):
> > 
> >         if response == gtk.RESPONSE_ACCEPT:
> >             settings.auto_load_images = dialog.auto_load_images.get_active()
> >             settings.rotation_mode = dialog.selector_orientation.get_active(0)
> >             settings.default_font_size = int(dialog.selector_font_size.get_current_text())
> >+            settings.user = dialog.entryUser.get_text()
> >+            settings.password = dialog.entryPassword.get_text()
> >             dialog.destroy()
> >+            if sync:
> >+                manager.sync_with_google_reader(settings.user, settings.password, self._feed_added_cb)
> > 
> >             # Update rotation
> >             self.rotation_object.set_mode(settings.rotation_mode)
> >@@ -672,7 +731,7 @@ class FeedsWindow(hildon.StackableWindow):
> >             row_reference = gtk.TreeRowReference(self.view.get_model(), path)
> > 
> >             # Add feed to manager
> >-            manager.add_feed(url, self._feed_added_cb, row_reference)
> >+            manager.add_feed(url, False, self._feed_added_cb, row_reference)
> > 
> >     def _found_cb(self, key_words, dialog, feeds_info):
> >         if feeds_info == None:
> >diff --git a/src/ReSiStance/settings.py b/src/ReSiStance/settings.py
> >index a127b54..8d4548b 100644
> >--- a/src/ReSiStance/settings.py
> >+++ b/src/ReSiStance/settings.py
> >@@ -40,6 +40,8 @@ class Settings(object):
> >         self.default_font_size = 16
> >         self.auto_load_images = True
> >         self.rotation_mode = FremantleRotation.AUTOMATIC
> >+        self.user  = ''
> >+        self.password = ''
> > 
> >     def load(self):
> >         if not (os.path.exists(constants.RSS_CONF_FILE) and
> >@@ -56,7 +58,15 @@ class Settings(object):
> >         self.default_font_size = self.config['default_font_size']
> >         self.auto_load_images = self.config['auto_load_images']
> >         self.rotation_mode = self.config['rotation_mode']
> >-
> >+        if 'user' in self.config:
> >+            self.user = self.config['user']
> >+        else:
> >+            self.config['user'] = ''
> >+        if 'password' in self.config:
> >+            self.password = self.config['password']
> >+        else:
> >+            self.config['password'] = ''
> 
> Use better the compact syntax added in python2.5
> 
> self.user = self.config['user'] if 'user' in self.config else ''
> self.password = self.config['password'] if 'password' in self.config else ''
> 
> >     def save(self):
> >         if self.config == None:
> >             self.config = ConfigObj(constants.RSS_CONF_FILE, configspec=self.spec)
> >@@ -66,5 +76,7 @@ class Settings(object):
> >         self.config['default_font_size'] = self.default_font_size
> >         self.config['auto_load_images'] = self.auto_load_images
> >         self.config['rotation_mode'] = self.rotation_mode
> >+        self.config['user'] = self.user
> >+        self.config['password'] = self.password
> > 
> >         self.config.write()
>
Comment 12 Chus (reporter) 2010-11-08 00:41:44 UTC
(In reply to comment #10)
> Another issue I have just discovered. I think the _preferences_response_cb is
> not correctly implemented. It has the sync default to False, this means that if
> you open the preferences dialog and add a user and a password it won't
> syncrhonize anything. What we should do in this case is pick the value from the
> new check button that will decide whether to synchronize or not and use that in
> that callback. Then you won't need to pass the sync as user_data. parametter.
> Obviously that sync boolean should be saved in the settings the same as the
> user and the password. That way user can add user and password and decide later
> not to synchronize
> 

The main menu have a button for synchronizing. If you want to put this check
button in SettingsDialog and sinchronize ReSiStance with Google Reader in this
dialog, the button "Synchronize" in the main menu is unnecessary. What must I
do?

> Also, you're not filling the user and password entries when the preferences
> dialog is opened again.
>
Comment 13 Sergio Villar Senin 2010-11-08 09:24:39 UTC
(In reply to comment #11)
> (In reply to comment #8)
> > A couple of issues with this dialog. 
> >    * It's better to create 2 hboxes, 1 for the username label and entry and
> > another one for the password.
> >    * Entries must be expanded till right border of the dialog.
> >    * Align labels to the left and do not include semicolons
> >    * When I test it, the label "Synchornize with google reader" is too short. I
> > think you have to set it finger height or something, because the letters are
> > not completely shown
> > 
> 
> I think that to align entries, I have to set a width. But, how can I do this? I
> find no suitable value (HILDON_SIZE_AUTO_WIDTH, HILDON_SIZE_HALFSCREEN_WIDTH,
> HILDON_SIZE_FULLSCREEN_WIDTH).

It's not a matter of setting a width, you just have to tell the entry to
expand/fill when adding it to its container (check gtk_box_pack_start
documentation)
Comment 14 Sergio Villar Senin 2010-11-08 09:35:50 UTC
(In reply to comment #12)
> (In reply to comment #10)
> > Another issue I have just discovered. I think the _preferences_response_cb is
> > not correctly implemented. It has the sync default to False, this means that if
> > you open the preferences dialog and add a user and a password it won't
> > syncrhonize anything. What we should do in this case is pick the value from the
> > new check button that will decide whether to synchronize or not and use that in
> > that callback. Then you won't need to pass the sync as user_data. parametter.
> > Obviously that sync boolean should be saved in the settings the same as the
> > user and the password. That way user can add user and password and decide later
> > not to synchronize
> > 
> 
> The main menu have a button for synchronizing. If you want to put this check
> button in SettingsDialog and sinchronize ReSiStance with Google Reader in this
> dialog, the button "Synchronize" in the main menu is unnecessary. What must I
> do?

Well, your current code does synchronize with google in the callback of that
dialog. What I think we should do is do the sync:
   1) in the callback of the settings dialog *if and only if* the user enabled
it in the check button and it was disabled before
   2) the user presses the "synchronize" option in the main menu

We could have only 2) but I think we need also 1) because in my humble opinion
users will expect a synchronization right after enabling it in the settings
dialog.
Comment 15 Chus (reporter) 2010-11-08 20:41:04 UTC
(In reply to comment #13)
> (In reply to comment #11)
> > (In reply to comment #8)
> > > A couple of issues with this dialog. 
> > >    * It's better to create 2 hboxes, 1 for the username label and entry and
> > > another one for the password.
> > >    * Entries must be expanded till right border of the dialog.
> > >    * Align labels to the left and do not include semicolons
> > >    * When I test it, the label "Synchornize with google reader" is too short. I
> > > think you have to set it finger height or something, because the letters are
> > > not completely shown
> > > 
> > 
> > I think that to align entries, I have to set a width. But, how can I do this? I
> > find no suitable value (HILDON_SIZE_AUTO_WIDTH, HILDON_SIZE_HALFSCREEN_WIDTH,
> > HILDON_SIZE_FULLSCREEN_WIDTH).
> 
> It's not a matter of setting a width, you just have to tell the entry to
> expand/fill when adding it to its container (check gtk_box_pack_start
> documentation)
> 

Yes, but with expand/fill, how can I align the entries?
Comment 16 Chus (reporter) 2010-11-08 21:23:36 UTC
Created an attachment (id=3165) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader.

SettingsDialog isn't complete. Entries aren't aligned. There is other problem,
the space in the dialog is little for all.
Comment 17 Sergio Villar Senin 2010-11-15 13:46:49 UTC
(In reply to comment #16)
> Created an attachment (id=3165) [details] [details]
> A patch to synchronize with Google Reader. It allows you to add the feeds that
> are in Google Reader.
> 
> SettingsDialog isn't complete. Entries aren't aligned. There is other problem,
> the space in the dialog is little for all.
> 

We're almost there. Just a couple of things left:

* You set the sync_global variable twice to True outside the settings dialog. I
think you should not do that.
* Synchronize button must not be shown if the sync_global setting is false
* There is no room left in the settings dialog: in order to fix that you have
to add a hildon.PannableArea to the settings dialog and then put all the
settings widgets inside the pannable instead of directly into the dialog.
* In order to have the user/password label and entries properly aligned you
have to create two GtkSizeGroup (with GTK_SIZE_GROUP_HORIZONTAL mode), one for
the entries and another one for the labels. Then add the 2 entries to one of
the groups and the labels to the other. That way entries will ask for the same
horizontal size, and labels will do the same.
Comment 18 Chus (reporter) 2010-11-17 01:09:46 UTC
Created an attachment (id=3196) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader.

You said "Synchronize button must not be shown if the sync_global setting is 
false". If sync_global is false, the user can synchronize with the 
settings dialog or with the main menu. I think that I must hide synchronize 
button if sync_global setting is true. Is this correct?
Comment 19 Sergio Villar Senin 2010-11-18 12:07:16 UTC
(In reply to comment #18)
> Created an attachment (id=3196) [details] [details]
> A patch to synchronize with Google Reader. It allows you to add the feeds that
> are in Google Reader.
> 
> You said "Synchronize button must not be shown if the sync_global setting is 
> false". If sync_global is false, the user can synchronize with the 
> settings dialog or with the main menu. I think that I must hide synchronize 
> button if sync_global setting is true. Is this correct?

Ah ok, you mean that in the future synchronization will happen automatically.
Ok then I agree that the synchronize button means "manual sync".

Several issues I detected:
  * try to rebase the patch agains the current master now that podcast patch is
committed
  * do not use the name "list" for the settings vbox, looks like a reserved
word
  * entryUser/entryPassword: replace with entry_user entry_password
  * user and password entries must be disabled if the check button is not
active, the same that happens with the download folder
  * Settings dialog is broken, you're still adding some settings to the dialog
vbox instead of to the pannable vbox
   * I think the synchronization is not correctly implemented for some reasons
      * it *always* add the google reader feeds in every sync. That means that
if I click on synchronize N times (I'll have number of feeds * N) feeds in
ReSiStance. We have to check that the feed is not already present.
      * I think the sync_with_google_reader method needs 2 callbacks. One that
will be called for each new feed (as the one you have now), but we need another
one in order to know when the synchronization ends, for example to turn off the
progress notification or whatever.
Comment 20 Chus (reporter) 2010-11-19 22:26:33 UTC
Created an attachment (id=3204) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader.
Comment 21 Chus (reporter) 2010-11-22 20:46:57 UTC
Created an attachment (id=3218) [details]
A patch to mark as read and as not read (from ReSiStance to Google Reader)
Comment 22 Sergio Villar Senin 2010-11-24 20:46:25 UTC
(From update of attachment 3204 [details])
Looks good. First of all you'll have to rebase it against the current master.
There are no settings or manager global objects among some other things.

The only remaining issue is that the code that tries to detect when a feed is
already setup does not work for most of the cases. The reason is that the
provided URL's are not normally the final url's. The server ends up redirecting
us to another one. So we have to find a way to check that. I'll think about it
Comment 23 Sergio Villar Senin 2010-11-29 11:28:52 UTC
(In reply to comment #21)
> Created an attachment (id=3218) [details] [details]
> A patch to mark as read and as not read (from ReSiStance to Google Reader)

Regarding read/unread patch:

   * The read_mark thing shouldn't be exposed to gui.py, it's a google specific
thing, so try to keep it inside the feedmanager.py. You can pass a boolean
instead and do the translation to read_mark in the synchronization function
   * I'm a bit worried about the network traffic. Would it be possible to send
google a list of entries in just one network call instead issuing them 1 by 1
?. The idea I have is that we could synchronize the headers after closing the
entries window. That way the user would read/unread some of them, and we'd
issue just one network call to google.
   * I don't understand the _connect_google callback. Isn't it always called
after the synchronization with google? If that's true then why are we always
showing an error?
Comment 24 Chus (reporter) 2010-12-01 01:58:12 UTC
(In reply to comment #23)
> (In reply to comment #21)
> > Created an attachment (id=3218) [details] [details] [details]
> > A patch to mark as read and as not read (from ReSiStance to Google Reader)
> 
> Regarding read/unread patch:
> 
>    * The read_mark thing shouldn't be exposed to gui.py, it's a google specific
> thing, so try to keep it inside the feedmanager.py. You can pass a boolean
> instead and do the translation to read_mark in the synchronization function
>    * I'm a bit worried about the network traffic. Would it be possible to send
> google a list of entries in just one network call instead issuing them 1 by 1
> ?. The idea I have is that we could synchronize the headers after closing the
> entries window. That way the user would read/unread some of them, and we'd
> issue just one network call to google.

I was checking the information about google reader (again) and I think that
there isn't a way to send a list of entries in one network call :( So I don't
know how to do what you say.

>    * I don't understand the _connect_google callback. Isn't it always called
> after the synchronization with google? If that's true then why are we always
> showing an error?
Comment 25 Sergio Villar Senin 2010-12-01 11:40:36 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > (In reply to comment #21)
> > > Created an attachment (id=3218) [details] [details] [details] [details]
> > > A patch to mark as read and as not read (from ReSiStance to Google Reader)
> > 
> > Regarding read/unread patch:
> > 
> >    * The read_mark thing shouldn't be exposed to gui.py, it's a google specific
> > thing, so try to keep it inside the feedmanager.py. You can pass a boolean
> > instead and do the translation to read_mark in the synchronization function
> >    * I'm a bit worried about the network traffic. Would it be possible to send
> > google a list of entries in just one network call instead issuing them 1 by 1
> > ?. The idea I have is that we could synchronize the headers after closing the
> > entries window. That way the user would read/unread some of them, and we'd
> > issue just one network call to google.
> 
> I was checking the information about google reader (again) and I think that
> there isn't a way to send a list of entries in one network call :( So I don't
> know how to do what you say.

Fair enough, if it isn't possible it is impossible :)
Comment 26 Chus (reporter) 2010-12-04 14:36:41 UTC
Created an attachment (id=3232) [details]
A patch to mark as read and as not read (from ReSiStance to Google Reader)

_connect_google callback isn't always called after the synchronization with
google, it is called only if we can't establish connection with google. I think
synchronize should be "transparent" to the user, so I don't show any
information when mark as read or as not read.
Comment 27 Chus (reporter) 2010-12-04 19:34:50 UTC
Created an attachment (id=3233) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader. (preparing release 0.7)
Comment 28 Chus (reporter) 2010-12-04 20:41:42 UTC
Created an attachment (id=3234) [details]
A patch to mark as read and as not read (from ReSiStance to Google Reader)
(preparing release 0.7)
Comment 29 Chus (reporter) 2010-12-04 22:59:49 UTC
Created an attachment (id=3235) [details]
A patch to synchronize a subscription (you can mark as sinchronized) and it is
going to appear in Google Reader. Something similar when you delete a
sinchronized feed. (preparing release 0.7)
Comment 30 Chus (reporter) 2010-12-05 14:06:57 UTC
Created an attachment (id=3236) [details]
A patch to synchronize with Google Reader. It allows you to add the feeds that
are in Google Reader. (preparing release 0.7)
Comment 31 Chus (reporter) 2010-12-06 05:08:05 UTC
Created an attachment (id=3237) [details]
A patch to synchronize automatically items with google.
Comment 32 Sergio Villar Senin 2010-12-19 12:49:14 UTC
(In reply to comment #30)
> Created an attachment (id=3236) [details] [details]
> A patch to synchronize with Google Reader. It allows you to add the feeds that
> are in Google Reader. (preparing release 0.7)

Committed.

I added a couple of changes after merging it in master. Basically the feed
manager just returns a list of urls. Then the UI will decide to add them or
not. That allows us to temporarily show dummy feeds in the Feeds Window while
the feeds are retrieved. This behaviour is more consistent with for example the
Find Feeds feature.
Comment 33 Sergio Villar Senin 2010-12-19 14:33:00 UTC
(In reply to comment #28)
> Created an attachment (id=3234) [details] [details]
> A patch to mark as read and as not read (from ReSiStance to Google Reader)
> (preparing release 0.7)

Committed.

I had to apply some changes before landing:
   * the "while True" was not properly implemented. Once we find the item we
must exit that loop. The break did only exit the for inner loop.
   * The item_ref passed to the sync method was not correct. We have to use
item.link
   * replaced connect_google by mark_as_read_sync_cb

Thx for the patch!!!
Comment 34 Sergio Villar Senin 2010-12-20 15:30:50 UTC
(In reply to comment #29)
> Created an attachment (id=3235) [details] [details]
> A patch to synchronize a subscription (you can mark as sinchronized) and it is
> going to appear in Google Reader. Something similar when you delete a
> sinchronized feed. (preparing release 0.7)

Committed.

I finally committed it as author (but properly specifying that was based on
your patch Chus) as I applied some heavy changes to the original patch:
   * do ALWAYS call callbacks if they exist. We must ensure the caller that the
callback will be always called
   * do not pass the action as a string to the feedmanager. That string is an
implementation detail of Google Reader. I replaced it by a boolean in the UI
code.
   * do not pass the username and the password. The feedmanager will get them
from settings
   * remove_feed method should not unsubscribe by default. The UI will decide
to do that or not. 
   * replaced the synchronize button in EntriesWindow by a button that does
subscribe or unsubscribe depending on the status of the feed. The label of the
button changes acordingly
   * removed the SynchronizeFeedsDialog. Makes no sense to add another dialog.
   * Added a confirmation dialog when removing a feed. The users need to
confirm if they want to remove the feed form Google Reader or not.
Comment 35 Sergio Villar Senin 2010-12-20 18:00:16 UTC
(In reply to comment #31)
> Created an attachment (id=3237) [details] [details]
> A patch to synchronize automatically items with google.

I have some doubts about this last patch.

First of all, when adding feeds from Google Reader (patch already committed) we
must use the function you have for comparing items, to properly set the
read/unread status of items. Having a timer for automatic updates could be a
good idea, but not only for Google Reader but for all feeds. Since we don't
have it right now I think it's better to remove it completely from this patch.
So my proposal would be:
 * use _read_items and _compare_items (or something similar) to properly set
the unread/read status of the items when adding feeds from Google Reader
(currently when you add something from Google Reader all items are unread which
is wrong)
 * synchronize the read/unread status the first time (in each execution of the
program) we visit a feed synchronized with google reader.

Having these two things is more than enough I think. Having a timer that
periodically updates the read/unread status is not that useful, because reading
things in the web while ReSiStance is opened is not a very typical use case.
The most common use cases:
* read/unread in ReSiStance -> update in Google Reader
* read/unread in Google Reader -> update in ReSiStance when opening a feed
will be perfectly covered with the changes I propose.
Comment 36 Sergio Villar Senin 2011-01-12 17:30:38 UTC
Closing the bug after 0.8.1 release