Re: [Rhythmbox-devel] [weekly report] Week 5 : Rhythmbox context pane

[Jonathan Matthew <jonathan d14n org>]

On Mon, Jun 29, 2009 at 05:10:35PM -0400, John Iacona wrote:
> This week I completed the datasource for fetching album info. This was
> more complicated than the artist datasource because it involved making
> a lot of different last.fm api calls to get all of the information.
> Handling all of the various connections and parsing of data
> asynchronously required some mucking about with idle callbacks and
> such to make sure all of the information was assembled correctly.
> 
> I did not get around to integrating the lyrics fetching code yet, I
> plan on doing that this week. For the mid-term evaluation, I would
> like to have all of the main data-fetching functionality done so that
> I can start polishing during the second half. Thus, this week, in
> addition to porting the lyrics code, I will work on cleaning up and
> making everything more robust.

I finally got around to looking at and attempting to run your code today.
Probably a bit slack of me.. anyway, some comments and suggestions:

- some of the template files are missing.  I can only see artist-tmpl.html
  in the git branch.
- I noticed you're using python 2.6 features in a few places, most noticeably
  string.format().  I think we need to keep compatibility with 2.5.
- I think it'd be worth looking into a full template system, rather than
  loading bits from multiple template files and gluing them together with
  other hardcoded bits.  The track list in the album info page would look
  something like this:

   <ul>
   {% for entry in tracklist %}
     <li>{{ entry.tracknumber }}.  {{ entry.title }} : {{ entry.duration }}</li>
   {% endfor %}
   </ul>

  rather than being a mixture of templates and code in different files.
     
- I'm not sure there's a need to write the generated html to disk for webkit to
  read it.  You can just pass the html to webview.load_html_string() instead.
  The current approach for finding filenames to use only works if the file already
  exists, and won't work if the plugin is installed somewhere the user can't write.