Re: [Rhythmbox-devel] Play Order Selection/Loop One Patch



On Thu, 2005-11-10 at 18:23 -0500, Paul Boyd wrote:
> Here is a patch to fix the play order selection problems. The check
> boxes have been removed from the status bar and the Control menu.
> There is a new Play order sub-menu under the Control menu that lists
> the available play orders. There is also the loop one play order that
> I submitted earlier. 

This looks good. This bring up the question of whether we should put a
play order control in the window. Personally, I don't change it often
enough to be worth it. If we used a combobox it would need to be
reasonable wide to fit the descriptions.


A couple of code comments:
* CONF_STATE_PLAY_ORDER is already be defined in rb-preferences.h, so
you don't need to do it in rb-shell.c

* In rb_shell_build_play_order_menu, you should use g_assert rather than
g_return_if_fail. That failing is a sign of something seriously being
wrong.


Somewhat related, we probably should also expose "set playorder by
name", "get playorder name" and "get playorder description" in the dbus
interface.


Cheers,

James "Doc" Livingston
-- 
We are sorry, but the number you have dialed is imaginary.
Please rotate your phone 90 degrees and try again.

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]