Re: [Rhythmbox-devel] Play Queue caption patch

[Jonathan Matthew <notverysmart gmail com>]

On Sat, Jul 26, 2008 at 11:41:48PM -0700, Kip Warner wrote:
> Greetings Everyone,
> 
> I've modified the Play Queue GUI to display the total play list time.
> When there are no tracks queued, the default is displayed as it was
> previously:
> 
> Play Queue
> 
> When there are tracks queued, rather than displaying something like
> this...
> 
> Play Queue (8)
> 
> ...it now displays...
> 
> Play Queue (8): 39 minutes, 28 seconds
> 
> I am receptive to criticism and I'd appreciate it if one of the
> maintainers could look over my patch for consideration to apply against
> svn head in trunk.

Please attach patches to bugs in bugzilla.gnome.org; otherwise, they
tend to be forgotten.

While I'm here, a few comments:

> Index: sources/rb-play-queue-source.c
> ===================================================================
> --- sources/rb-play-queue-source.c	(revision 5821)
> +++ sources/rb-play-queue-source.c	(working copy)
> @@ -457,28 +457,89 @@
>  				   GtkTreeModel *model,
>  				   gint offset)
>  {
> -	gint count = gtk_tree_model_iter_n_children (model, NULL) + offset;
> -	RBPlayQueueSourcePrivate *priv = RB_PLAY_QUEUE_SOURCE_GET_PRIVATE (source);
> -	char *name = _("Play Queue");
> -	GtkAction *action;
>  
> -	/* update source name */
> -	if (count > 0)
> -		name = g_strdup_printf ("%s (%d)", name, count);
> +        gint count = gtk_tree_model_iter_n_children (model, NULL) + offset;
> +        RBPlayQueueSourcePrivate *priv = RB_PLAY_QUEUE_SOURCE_GET_PRIVATE (source);
> +        char *name = _("Play Queue");
> +        char *pszTemp = NULL;

We don't use hungarian notation..

> +        GtkAction *action;
>  
> -	g_object_set (G_OBJECT (source), "name", name, NULL);
> -	gtk_tree_view_column_set_title (priv->sidebar_column, name);
> +        // There are items available, update name to reflect this...

.. or c++-style comments.

> +        if (count > 0) {
>  
> -	if (count > 0)
> -		g_free (name);
> +            // Compute and format components of total duration needed later...
>  
> +                // Total duration...
> +                glong const lTotalDuration = 
> +                rhythmdb_query_model_get_duration(RHYTHMDB_QUERY_MODEL(model));

Most of the rest of the code from here on was more or less identical to
that in rhythmdb_compute_status_normal.  Rather than adding another
mostly-identical copy, it should be extracted out into a utility
function somewhere.