Re: [Rhythmbox-devel] Lyrics Plugin

[Steven Walter <stevenrwalter gmail com>]

On Thu, May 03, 2007 at 08:38:03PM -0500, Sirio Bolaños Puchet wrote:
> I really just copied code from amarok sources, translated it from ruby
> to python (not that hard...), and the existing one from rhythmbox
> sources and added up some little things.
> 
> I don't know very well how to code in python nor pygtk, I just learned
> by example from other code and the pygtk tut, so maybe its not very
> efficient or something, but it works good, at least for me.

Since you stated you were not an experience python coder, I decided to
look over your code and clean up a few places that were needlessly
complex.  Overall, the code looks pretty good.  Patch of suggested
changes is attached.
-- 
-Steven Walter <stevenrwalter gmail com>

>From fef9557707ccb19538c41e50de599e2549611d5b Mon Sep 17 00:00:00 2001
From: Steven Walter <stevenrwalter gmail com>
Date: Mon, 7 May 2007 14:01:28 -0400
Subject: [PATCH] Code cleanups
Status: O
Content-Length: 5234
Lines: 159

---
 lyrics.py |   26 ++++++--------------------
 parse.py  |   43 ++++++++++++++++++-------------------------
 2 files changed, 24 insertions(+), 45 deletions(-)

diff --git a/lyrics.py b/lyrics.py
index c9e5588..cac8c95 100644
--- a/lyrics.py
+++ b/lyrics.py
@@ -39,16 +39,13 @@ def parse_song_data(artist,title):
     
     # replace ampersands and the like
     for exp in LYRIC_ARTIST_REPLACE:
-        p = re.compile (exp[0])
-        artist = p.sub(exp[1], artist)
+        artist = re.sub(exp[0], exp[1], artist)
     for exp in LYRIC_TITLE_REPLACE:
-        p = re.compile (exp[0])
-        title = p.sub(exp[1], title)
+        title = re.sub(exp[0], exp[1], title)
 
     # strip things like "(live at Somewhere)", "(accoustic)", etc
     for exp in LYRIC_TITLE_STRIP:
-        p = re.compile (exp)
-        title = p.sub ('', title)
+        title = re.sub (exp, '', title)
 
     # compress spaces
     title = title.strip()
@@ -57,7 +54,6 @@ def parse_song_data(artist,title):
     return (artist,title)
     
 def build_cache_path(artist, title):
-
     lyrics_folder = os.path.expanduser (LYRICS_FOLDER)
     if not os.path.exists (lyrics_folder):
         os.mkdir (lyrics_folder)
@@ -69,9 +65,7 @@ def build_cache_path(artist, title):
     return artist_folder + '/' + title[:128] + '.lyric'
 
 class LyricGrabber(object):
-
     def __init__(self,db,entry):
-        
         self.loader = rb.Loader ()
         self.db = db
         self.entry = entry
@@ -84,33 +78,25 @@ class LyricGrabber(object):
         self.cache_path = build_cache_path(self.artist,self.title)
 
     def verify_lyric(self):
-        
-        if os.path.exists (self.cache_path):
-            return True
-        else:
-            return False
+        return os.path.exists (self.cache_path)
       
     def search_lyrics(self,callback):
         self.callback = callback
         
         status = self.verify_lyric()
         
-        if status == True:
+        if status:
             self.loader.get_url(self.cache_path, callback)
-        elif status == False:
+        else:
             parser = parse.Parser(self.artist,self.title)
             text = parser.return_lyric()
             f = file (self.cache_path, 'w')
             f.write (text)
             f.close ()
             self.callback(text)
-        else:
-            return
-        
 
 class LyricPane(object):
     def __init__(self, db, song_info):
-        
         self.db = db
         self.song_info = song_info
         self.entry = self.song_info.get_property("current-entry")
diff --git a/parse.py b/parse.py
index b65e470..503530a 100644
--- a/parse.py
+++ b/parse.py
@@ -23,18 +23,17 @@ class Parser (object):
         astraweb_parser = Astraweb_parser(self.artist,self.title)
         
         leoslyrics_lyric = leoslyrics_parser.parse_results()      
-        if leoslyrics_lyric is not False:
+        if leoslyrics_lyric:
             return leoslyrics_lyric
-        else:
-            lyrc_lyric = lyrc_parser.parse_results()
-            if lyrc_lyric is not False:
-                return lyrc_lyric
-            else:
-                astraweb_lyric = astraweb_parser.parse_results()
-                if astraweb_lyric is not False:
-                    return astraweb_lyric
-                else:
-                    return "No suitable lyric found."
+
+        lyrc_lyric = lyrc_parser.parse_results()
+        if lyrc_lyric:
+            return lyrc_lyric
+
+        astraweb_lyric = astraweb_parser.parse_results()
+        if astraweb_lyric:
+            return astraweb_lyric
+        return "No suitable lyric found."
             
 class Lyrc_parser (object):
 
@@ -123,16 +122,13 @@ class Astraweb_parser (object):
                 if not ((re.search(self.title.lower().strip(),title.lower().strip()) is None)):
                     if not (re.search(self.artist.lower().strip(),artist.lower().strip()) is None):
                         return self.parse_lyrics(url)
-                    else:
-                        continue
-                else:
                     continue
-            
-            return False
+                continue
 
-        else:
             return False
 
+        return False
+
     def parse_lyrics(self,url):
         
         path = 'http://display.lyrics.astraweb.com'
@@ -192,14 +188,11 @@ class Leoslyrics_parser(object):
             title = match.getElementsByTagName('title')[0].firstChild.data
             artist = match.getElementsByTagName('name')[0].firstChild.data
             
-            if not ((re.search(self.title.lower().strip(),title.lower().strip()) is None)):
-                if ((re.search(self.artist.lower().strip(),artist.lower().strip()) is None)):
-                    matches = matches[i:]
-                else:
-                    continue
-            else:
-                matches = matches[i:]
-            
+            if (re.search(self.title.lower().strip(),title.lower().strip()) and
+                    re.search(self.artist.lower().strip(),artist.lower().strip())):
+                continue
+
+            matches = matches[i:]
             i += 1
         
         hids = map(lambda x: x.getAttribute('hid'), matches)
-- 
1.5.1.2