Turning on Jump to(focussed) if not using dynamic playlists via config file

3 views
Skip to first unread message

Rajiv Nair

unread,
Dec 2, 2009, 12:26:16 PM12/2/09
to rem...@googlegroups.com
I use remuco-amarok to link upto my laptop from my phone. But i
noticed that the "Jump To(focussed)" playlist option is disabled since
its unambiguous for dynamic playlists. But when I'm not using dynamic
playlists in amarok its kind of a pain to navigate to a song in a
playlist whose size is greater than say 10, because I have to keep
pressing "Next track" to reach the song I want. Hence I added a config
option to check whether a user uses dynamic-playlists on his player
and if it says "no", the "Jump to" option gets enabled in the list of
playlist actions. I've pushed the changes into my Google code clone.
Can somebody review this?

--
Rajiv R Nair

"Every one of us is precious in the cosmic perspective. If a human
disagrees with you, let him live. In a hundred billion galaxies, you
will not find another. - Carl Sagan"

Oben Sonne

unread,
Dec 2, 2009, 3:05:49 PM12/2/09
to rem...@googlegroups.com
Looks good. I would name the config option 'mpris_jump' and when it is
set to true, the jump action gets enabled. Further, the value should
be 0 or 1, not yes or no, and the property get function should return
a boolean, like for the properties 'bluetooth' or 'wifi'. The
__pget_...() function of the property should have a docstring similar
to the other property __get_..() functions.

Cheers,
Oben
> --
>
> You received this message because you are subscribed to the Google Groups "remuco" group.
> To post to this group, send email to rem...@googlegroups.com.
> To unsubscribe from this group, send email to remuco+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/remuco?hl=en.
>
>
>

Rajiv Nair

unread,
Dec 2, 2009, 3:29:20 PM12/2/09
to rem...@googlegroups.com
I've made the necessary changes and have pushed them into my clone.
Thanks a million for the pointers :)

Regards,
Rajiv

Oben Sonne

unread,
Dec 2, 2009, 6:12:41 PM12/2/09
to rem...@googlegroups.com
Better, but still not finished ;)

The __pget function still returns "no" when there are config errors
(should return 0). Further the default value for this method should be
0 (disabled) - otherwise users who don't know of this option might get
confused. Finally the docstring is still missing.

When done:

Before I pull your changes, please combine the MPRIS related commits
to a single one. Here you can use the collapse and/or rebase and/or
transplant extension or, maybe easier, by creating a diff for the
mentioned revisions. The combined change should then be applied to the
revision before 12d36eb714 (the image workaround, would like to
exclude this for now), this will create a new head in your repo - the
one I want to pull. When writing the commit message for the combined
patch please follow the guide lines for commit messages
(http://code.google.com/p/remuco/wiki/Contribute).

Rajiv Nair

unread,
Dec 3, 2009, 12:13:54 PM12/3/09
to rem...@googlegroups.com
Done :). I've pushed just the config file changes as a single commit
as a new head in my repo. Revision no. 9f2e3f7d8b. Hope I've gotten it
right this time ;)

Oben Sonne

unread,
Dec 4, 2009, 6:55:16 PM12/4/09
to rem...@googlegroups.com
Merged: http://code.google.com/p/remuco/source/detail?r=2b05faa41599c885875e17e186ec6804849bbd71

In the future, please ensure to use 4 spaces instead of tabs (Python
prefers spaces, mixing tabs and spaces causes problems because in
Python indentations are part of the syntax).

Thanks for the patch!

Oben
Reply all
Reply to author
Forward
0 new messages