Re: Handling of compilations

[Gregory P. <gre...@gm...>]

On Sun, Jan 24, 2010 at 05:49:35PM +0100, Michael Piotrowski wrote:
> +	/*
> +	 * Sort regular albums by date, but sort compilations
> +	 * alphabetically.
> +	 */
> +	if (u_strcasecmp(artist->name, "<Compilations>") != 0) {

I think it's better to pass is-compilation flag to this function (to not have
this magic '<Compilations>' string here).

You can reuse result of track_info_is_compilation() call in tree_add_track().

> +		list_for_each(item, &artist->album_head) {
> +			struct album *a = to_album(item);
>  
> -		if (date < a->date)
> -			break;
> -		if (date > a->date)
> -			continue;
> -		if (special_name_cmp(name, a->name) < 0)
> -			break;
> +			if (date < a->date)
> +				break;
> +			if (date > a->date)
> +				continue;
> +			if (special_name_cmp(name, a->name) < 0)
> +				break;
> +		}
> +	} else {
> +		list_for_each(item, &artist->album_head) {
> +			struct album *a = to_album(item);

Nitpick: empty line here will be good.

> +			if (special_name_cmp(name, a->name) < 0)
> +				break;
> +		}
>  	}
> +
>  	/* add before item */
>  	list_add_tail(&album->node, item);
>  	return album;

> I could try to provide a git patch, if this is preferred.

Yes, git format is preferred. It would be great if you can include
part of your explanation in the commit message (and add man page changes to
the commit as well).

	Gregory