GNOME Bugzilla – Bug 683375
add shuffle toggle command line interface
Last modified: 2012-10-30 13:57:52 UTC
Created attachment 223483 [details] rb-shell.c.diff and rb-client.c.diff to apply changes Add's support for activation and deactivation of shuffle via command line. useful for other applications that use rhythmbox-client interface to control rhythmbox
please attach patches in 'git diff' format.
Created attachment 224717 [details] [review] like this
Review of attachment 224717 [details] [review]: Sorry, I gave you the wrong advice on how to implement this. The MPRIS spec includes a Shuffle property that can be used to control this, so we don't need to add a new action to the shell. See how --set-volume and --volume-up/down work to see how to use MPRIS properties. If we're going to have a way to toggle shuffle, we should also have a way to do repeat. ::: remote/dbus/rb-client.c @@ +96,3 @@ /* { "stop", 0, 0, G_OPTION_ARG_NONE, &stop, N_("Stop playback"), NULL }, */ + { "toggle-shuffle", 0, 0, G_OPTION_ARG_NONE, &toggle_shuffle, N_("Toggle on/off shuffle mode"), NULL }, 'toggle' already implies 'on/off', no need to say it again @@ +750,3 @@ } + + /* shuffle mode needs to be determined before switching track */ I don't think this comment is terribly helpful
Created attachment 225239 [details] [review] adds --set-repeat and --set-shuffle to rhythmbox-client fixes the problems issued in the first submission attempt.
Created attachment 225250 [details] [review] also adds --set-repeat and --set-shuffle to rhythmbox-client without querying their current state This is just a alternative to the other patch since querying current status of repeat and shuffle is pretty much useless unless the interface contained --set-repeat=on/off/toggle --set-shuffle=on/off/toggle Anyways feel free to use any of those.
Review of attachment 225250 [details] [review]: ::: remote/dbus/rb-client.c @@ +108,3 @@ + { "set-repeat", 0, 0, G_OPTION_ARG_STRING, &set_repeat, N_("Set repeat state on/off"), N_("on/off") }, + { "set-shuffle", 0, 0, G_OPTION_ARG_STRING, &set_shuffle, N_("Set shuffle state on/off"), N_("on/off") }, I'd rather we had --shuffle and --no-shuffle, --repeat and --no-repeat. That way we don't have to deal with translations for on/off, or people expecting 1/0, yes/no and true/false to work. @@ +858,3 @@ + repeat_state = "None"; + run_dbus_call = TRUE; + } If we go with "--set-shuffle on", you'd need an error message here in the case where the user specifies something else, rather than just failing silently.
Created attachment 225419 [details] [review] add repeat, no-repeat, shuffle and no-shuffle to rhythmbox-client This patch introduces the proposed changes of the last review https://bugzilla.gnome.org/review?bug=683375&attachment=225250
Cleaned up a few things and pushed as commit 96bc197. Thanks for the patch.