Merge lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras

[Gabor Karsay]

onox, you have more experience in python, could you please have a look at this method before I add it to awnlib.py? Is it okay for you, e.g. description-wise? As you'll see, it tries to unlock the gnome keyring, with a little workaround though.

It fixes bug #581075, probably bug #549246. I don't know why, but it also fixes bug #610088 (the Gmail part).

btw, how do you decide which bugs are milestones?

[Alberto Aldegheri (albyrock87)]

Forgive me but I don't understand one thing in unlock() method:

- line 43: if "info.get_is_locked()" is False, it jumps to line 59 and returns <<True>>
- line 57: if "info.get_is_locked()" is False, it returns <<False>>

Why?

[Gabor Karsay (gabor-karsay)]

You are right, that is a mistake. But in a test it works as it should, so I guess that the lock state is not updated and it uses the old one, updating soon.

[lp:~gabor-karsay/awn-extras/various-fixes]

1415. By Gabor Karsay on 2010-09-14

Fix logic mistake, thanks alberto

[onox (onox)]

Gabor, it seems the linked bug reports on this page are different from those in your description. Could you fix this? Please try to avoid reusing some branch for different things. (It's good, however, that you use branches, allows people to review the code :)) Furthermore, about those bugs you described, set them to "Confirmed" once you can reproduce them. Their status still seems to be "New". Use "In Progress" if you're working on them.

About the milestones: some people set bugs to milestone 0.4.2 if the bug occurs in a previous version and will be fixed in 0.4.2. Personally I use 0.4.2 whenever the fix ends up in 0.4.2, this including bugs that were introduced in between 0.4.0 and 0.4.2 (0.4.1 / trunk)

About the code: I haven't tested it (because I don't (want) / can't use the mail applet) so I don't know whether the changes are functional correct. One thing you could do, however, is change def unlock to:

if not info.get_is_locked():
    return True

# here the indented code

return not info.get_is_locked()

This way, the most important piece of code of the function is 1 indention less. One could argue it's a little big more readable.
Oh, and you could replace the url to "launchpad bug #432882"

[onox (onox)]

And make sure the code is PEP8 compliant.

On Tue, Sep 14, 2010 at 10:47 PM, onox <email address hidden> wrote:

> Gabor, it seems the linked bug reports on this page are different from
> those in your description. Could you fix this? Please try to avoid reusing
> some branch for different things. (It's good, however, that you use
> branches, allows people to review the code :)) Furthermore, about those bugs
> you described, set them to "Confirmed" once you can reproduce them. Their
> status still seems to be "New". Use "In Progress" if you're working on them.
>
> About the milestones: some people set bugs to milestone 0.4.2 if the bug
> occurs in a previous version and will be fixed in 0.4.2. Personally I use
> 0.4.2 whenever the fix ends up in 0.4.2, this including bugs that were
> introduced in between 0.4.0 and 0.4.2 (0.4.1 / trunk)
>
> About the code: I haven't tested it (because I don't (want) / can't use the
> mail applet) so I don't know whether the changes are functional correct. One
> thing you could do, however, is change def unlock to:
>
> if not info.get_is_locked():
> return True
>
> # here the indented code
>
> return not info.get_is_locked()
>
> This way, the most important piece of code of the function is 1 indention
> less. One could argue it's a little big more readable.
> Oh, and you could replace the url to "launchpad bug #432882"
> --
>
> https://code.launchpad.net/~gabor-karsay/awn-extras/various-fixes/+merge/35450
> You are requested to review the proposed merge of
> lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras.
>

[Alberto Aldegheri (albyrock87)]

Onox: can this help? http://pypi.python.org/pypi/pep8

[onox (onox)]

http://wiki.awn-project.org/Applets:DevelopmentGuidelines#Python_PEP_8

On Tue, Sep 14, 2010 at 11:08 PM, Alberto
<<email address hidden><albyrock87%<email address hidden>>
> wrote:

> Onox: can this help? http://pypi.python.org/pypi/pep8
> --
>
> https://code.launchpad.net/~gabor-karsay/awn-extras/various-fixes/+merge/35450
> You are requested to review the proposed merge of
> lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras.
>

[onox (onox)]

@Gabor: see launchpad bug #432957. Seems related to this branch also.

On Tue, Sep 14, 2010 at 11:28 PM, onox <email address hidden> wrote:

> http://wiki.awn-project.org/Applets:DevelopmentGuidelines#Python_PEP_8
>
> On Tue, Sep 14, 2010 at 11:08 PM, Alberto
> <<email address hidden> <albyrock87%<email address hidden>><
> albyrock87%<email address hidden> <albyrock87%<email address hidden>>>
> > wrote:
>
> > Onox: can this help? http://pypi.python.org/pypi/pep8
> > --
> >
> >
> https://code.launchpad.net/~gabor-karsay/awn-extras/various-fixes/+merge/35450
> > You are requested to review the proposed merge of
> > lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras.
> >
>
> --
>
> https://code.launchpad.net/~gabor-karsay/awn-extras/various-fixes/+merge/35450
> You are requested to review the proposed merge of
> lp:~gabor-karsay/awn-extras/various-fixes into lp:awn-extras.
>

[lp:~gabor-karsay/awn-extras/various-fixes]

1416. By Gabor Karsay on 2010-09-15

Change as reviewed by onox, make awnlib.py PEP 8 compliant

1417. By Gabor Karsay on 2010-09-15

Make mail.py PEP 8 compliant

[Gabor Karsay (gabor-karsay)]

Thanks Alberto, thanks onox, applied all your suggestions. The code is tested and functional. I go ahead and commit it now.

[onox (onox)]

review: Approve

=== modified file 'applets/maintained/mail/mail.py'
--- applets/maintained/mail/mail.py	2010-09-13 16:49:15 +0000
+++ applets/maintained/mail/mail.py	2010-09-15 19:43:46 +0000
@@ -83,7 +83,7 @@
         """
         Login. Try to login from saved key, if this does not exist or
         force is True, show login dialog
-        
+
         """
         self.awn.theme.icon("login")
 
@@ -107,6 +107,9 @@
 
         key = self.awn.keyring.from_token(token)
 
+        if not self.awn.keyring.unlock():
+            return self.login(True)
+
         self.perform_login(key)
 
     def logout(self):
@@ -116,9 +119,8 @@
         self.awn.settings["login-token"] = 0
 
     def perform_login(self, key):
-#        if key.token == 0:
-#            self.__dialog.login_form(True, "Both username and password must be specified.")
-#            return
+        if key.token == 0:
+            return
 
         try:
             self.mail = self.back(key)  # Login
@@ -210,11 +212,11 @@
 
         """
         states = {
-            "error" : "error",
-            "login" : "login",
-            "read"  : "mail-read",
-            "unread": "mail-unread"
-        }
+            "error": "error",
+            "login": "login",
+            "read": "mail-read",
+            "unread": "mail-unread"}
+
         self.awn.theme.set_states(states)
         theme = self.awn.settings["theme"] if self.awn.settings["theme"] != system_theme_name else None
         self.awn.theme.theme(theme)
@@ -280,7 +282,7 @@
     def email_list(self):
         """
         Creates a dialog with mail subjects and 3-4 buttons
-        
+
         """
         self.__remove_current()
         self.__current_type = "email_list"
@@ -301,7 +303,7 @@
 
             # This'll be the "show web interface" button
             b = gtk.Button()
-            b.set_relief(gtk.RELIEF_NONE) # Found it; that's a relief
+            b.set_relief(gtk.RELIEF_NONE)  # Found it; that's a relief
             b.set_image(gtk.image_new_from_stock(gtk.STOCK_NETWORK,
                                                  gtk.ICON_SIZE_BUTTON))
             b.set_tooltip_text(_("Open Web Mail"))
@@ -405,7 +407,7 @@
     def login_form(self, error=False, message=_("Wrong username or password")):
         """
         Creates a dialog the login form
-        
+
         """
         self.__remove_current()
         self.__current_type = "login_form"
@@ -473,6 +475,7 @@
                                                gtk.ICON_SIZE_BUTTON)
         submit_button = gtk.Button(label=_("Log In"), use_underline=False)
         submit_button.set_image(image_login)
+
         def onsubmit(widget):
             self.__parent.perform_login(
                                 t["callback"](t["widgets"], self.__parent.awn))
@@ -506,7 +509,7 @@
 
         def update(self):
 
-            if not self.key.attrs.has_key("username"):
+            if not "username" in self.key.attrs:
                 raise RuntimeError(_("Could not log in: No username"))
                 return
 
@@ -528,8 +531,8 @@
                 self.subjects.append(i.title)
 
         def __cleanGmailSubject(self, n):
-            n = re.sub(r"^[^>]*\\>", "", n) # "sadf\>fdas" -> "fdas"
-            n = re.sub(r"\\[^>]*\\>$", "", n) # "asdf\afdsasdf\>" -> "asdf"
+            n = re.sub(r"^[^>]*\\>", "", n)  # "sadf\>fdas" -> "fdas"
+            n = re.sub(r"\\[^>]*\\>$", "", n)  # "asdf\afdsasdf\>" -> "asdf"
             n = n.replace(""", "\"")
             n = n.replace("&", "&")
             n = n.replace(" ", "")
@@ -543,7 +546,7 @@
         def __cleanMsg(self, n):
             n = re.sub("\n\s*\n", "\n", n)
             n = re.sub("&[#x(0x)]?\w*;", " ", n)
-            n = re.sub("\<[^\<\>]*?\>", "", n) # "<h>asdf<a></h>" -> "asdf"
+            n = re.sub("\<[^\<\>]*?\>", "", n)  # "<h>asdf<a></h>" -> "asdf"
 
             f = False
             h = []
@@ -569,8 +572,8 @@
 
         def update(self):
 
-            if not self.key.attrs.has_key("username") or \
-               not self.key.attrs.has_key("domain"):
+            if not "username" in self.key.attrs or \
+               not "domain" in self.key.attrs:
                 raise RuntimeError(_("Could not log in: No username or domain"))
                 return
 
@@ -593,8 +596,8 @@
                 self.subjects.append(i.title)
 
         def __cleanGmailSubject(self, n):
-            n = re.sub(r"^[^>]*\\>", "", n) # "sadf\>fdas" -> "fdas"
-            n = re.sub(r"\\[^>]*\\>$", "", n) # "asdf\afdsasdf\>" -> "asdf"
+            n = re.sub(r"^[^>]*\\>", "", n)  # "sadf\>fdas" -> "fdas"
+            n = re.sub(r"\\[^>]*\\>$", "", n)  # "asdf\afdsasdf\>" -> "asdf"
             n = n.replace("&quot;", "\"")
             n = n.replace("&amp;", "&")
             n = n.replace("&nbsp;", "")
@@ -608,7 +611,7 @@
         def __cleanMsg(self, n):
             n = re.sub("\n\s*\n", "\n", n)
             n = re.sub("&[#x(0x)]?\w*;", " ", n)
-            n = re.sub("\<[^\<\>]*?\>", "", n) # "<h>asdf<a></h>" -> "asdf"
+            n = re.sub("\<[^\<\>]*?\>", "", n)  # "<h>asdf<a></h>" -> "asdf"
 
             f = False
             h = []
@@ -653,6 +656,7 @@
     except:
         pass
     else:
+
         class UnixSpool:
 
             title = _("Unix Spool")
@@ -699,6 +703,7 @@
     except:
         pass
     else:
+
         class POP:
 
             title = "POP"
@@ -715,7 +720,7 @@
                     raise RuntimeError(_("Could not log in: ") + str(message))
 
                 else:
-                    if not key.attrs.has_key("username"):
+                    if not "username" in key.attrs:
                         raise RuntimeError(_("Could not log in: No username"))
                     self.server.user(key.attrs["username"])
                     try:
@@ -787,6 +792,7 @@
     except:
         pass
     else:
+
         class IMAP:
 
             title = "IMAP"
@@ -823,7 +829,7 @@
                         s = self.server.fetch(i, '(BODY[HEADER.FIELDS (SUBJECT)])')[1][0]
 
                         if s is not None:
-                            self.subjects.append(s[1][9:].replace("\r\n", "\n").replace("\n", "")) # Don't ask
+                            self.subjects.append(s[1][9:].replace("\r\n", "\n").replace("\n", ""))  # Don't ask
                 else:
                     mboxs = [re.search("(\W*) (\W*) (.*)", i).groups()[2] for i in self.server.list()[1]]
                     mboxs = [i for i in mboxs if i not in ("Sent", "Trash") and i[:6] != "[Gmail]"]
@@ -843,7 +849,7 @@
                             s = self.server.fetch(i, '(BODY[HEADER.FIELDS (SUBJECT)])')[1][0]
 
                             if s is not None:
-                                self.subjects.append(s[1][9:].replace("\r\n", "\n").replace("\n", "")) # Don't ask
+                                self.subjects.append(s[1][9:].replace("\r\n", "\n").replace("\n", ""))  # Don't ask
 
             @classmethod
             def drawLoginWindow(cls, *groups):

=== modified file 'shared/python/awnlib.py'
--- shared/python/awnlib.py	2010-09-09 21:56:36 +0000
+++ shared/python/awnlib.py	2010-09-15 19:43:46 +0000
@@ -568,6 +568,7 @@
 
     def set_error_icon_and_click_to_restart(self):
         self.__parent.icon.theme("dialog-error")
+
         def crash_applet(widget=None, event=None):
             gtk.main_quit()
         self.__parent.connect("clicked", crash_applet)
@@ -594,7 +595,7 @@
             error_type = type(error).__name__
             error = str(error)
             if traceback is not None:
-                print "\n".join(["-"*80, traceback, "-"*80])
+                print "\n".join(["-" * 80, traceback, "-" * 80])
                 summary = "%s in %s: %s" % (error_type, self.__parent.meta["name"], error)
                 if self.__parent.meta["version"] == __version__:
                     args["message"] = "Visit Launchpad and report the bug by following these steps:\n\n" \
@@ -637,6 +638,7 @@
             copy_summary_button.connect("clicked", clicked_cb, summary)
 
         if callable(callback):
+
             def response_cb(widget, response):
                 if response < 0:
                     callback()
@@ -782,7 +784,7 @@
 
             """
             self.__config_object = None
-            
+
             type_client = type(client)
             if client is None:
                 self.__client = awn.config_get_default(awn.PANEL_ID_DEFAULT)
@@ -932,6 +934,33 @@
         k.token = token
         return k
 
+    def unlock(self):
+        """Unlock keyring.
+
+        @return: True if keyring is unlocked, False if locked.
+
+        """
+
+        info = gnomekeyring.get_info_sync(None)
+        if not info.get_is_locked():
+            return True
+
+        # The straight way would be:
+        # gnomekeyring.unlock_sync(None, None)
+        # But this results in a type error, see launchpad bugs #432882.
+        # We set a dummy key instead and delete it immediately,
+        # this triggers a user dialog to unlock the keyring.
+        try:
+            tmp = gnomekeyring.item_create_sync(None, \
+                  gnomekeyring.ITEM_GENERIC_SECRET, "awn-extras dummy", \
+                  {"dummy_attr": "none"}, "dummy_pwd", True)
+        except gnomekeyring.CancelledError:
+            return False
+        gnomekeyring.item_delete_sync(None, tmp)
+
+        info = gnomekeyring.get_info_sync(None)
+        return not info.get_is_locked()
+
     class Key(object):
 
         def __init__(self, token=0):
@@ -966,8 +995,11 @@
             else:  # Generic included
                 type = gnomekeyring.ITEM_GENERIC_SECRET
 
-            self.token = gnomekeyring.item_create_sync(None, type, name, \
-                attrs, pwd, True)
+            try:
+                self.token = gnomekeyring.item_create_sync(None, type, name, \
+                    attrs, pwd, True)
+            except gnomekeyring.CancelledError:
+                self.token = 0
 
         def delete(self):
             """Delete the current key. Will also reset the token. Note that
@@ -1220,7 +1252,7 @@
                 self.__notification.set_timeout(timeout * 1000)
 
         def show(self):
-            self.__notification.show()            
+            self.__notification.show()
 
 
 class Meta: