Status report on provider-based menus

[Steve Dodier-Lazaro]

This is a status report on porting all menus and plugin menu items to providers.

I've had to make a few changes to xlgui.widgets.menu.py, allowing check_menu_items to have callback arguments (that will be used both for the function that checks if the widget should be enabled and for the callback of the item). Otherwise, users would have to subclass Menuitem each time they want to use a flag in an object for their "checked_func" (because the default parameters, name/parent/context are mostly useless in determining whether the check menu item should be checked or not).

Also, I wrote functions for adding MenuItems and CheckMenuItems to the Tools menu directly. There are two reasons for this decision:
 - Appending an item to the tools menu is not possible without setting the "after" parameter correctly. Instead of asking plugin developers to find the name of the last item, I setup a plugins-separator, and my functions always insert items after this separator (so they're not exactly appended but at least they don't get in the way of the default menu items). I still need to make it possible to hide the separator based on the presence of items after it, but I don't know how to do that yet.
 - It is more than time to write an API for inserting menu items in the tools menu, as there are currently a lot of plugins to port, and I wouldn't want any future changes to the internal menu items API to mean that all of these plugins are to be ported again. Ideally, there should be some xl.plugin_utils.py and xlgui.plugin_utils.py files.

Currently, only the shutdown and ipconsole plugins are ported, so this is still work in progress. I'm posting a diff only for developers to be able to point to any potential (and likely) mistakes, but please do not commit it yet.

[reacocard (reacocard)]

> I've had to make a few changes to xlgui.widgets.menu.py, allowing
> check_menu_items to have callback arguments
Good.

> name/parent/context are mostly useless ... checked or not
Not true. Name is in there specifically because its useful for letting you use the same checked_func for multiple items in the same group, and parent/context can be used as the source of information for determining the checked state. This is used already in the menu for selecting visible playlist columns, and the menus on the repeat and shuffle mode toggles.

> It is more than time to write an API for inserting menu items in the
> tools menu,
The idea with moving to providers is that ALL items should be able to be added to ANY menu by the same API, regardless of their origin. I will _not_ accept patches that hack around this, instead we need to expand the functionality of the after-based API to be able to handle these cases.

> Appending an item to the tools menu is not possible without setting
> the "after" parameter correctly. Instead of asking plugin developers
> to find the name of the last item, I setup a plugins-separator....
This is a step in the right direction to solve this end of the case - perhaps simply allowing for some sort of 'placeholder' items that don't show in the display but can be used in after= designations is a good way to fix this limitation. Probably also worth thinking about adding a similar placeholder at the beginning of the menus, so items can be easily prepended as well as appended.

> I wouldn't want any future changes ... to mean that all of these
> plugins are to be ported again.
Uh, have you looked at our dev history? We re-port plugins ALL THE TIME. Its a simple side effect of not having a reasonably stable internal API, and until we do, that's always going to be the case. Best way to solve this for now is to just try to make our APIs as good as possible so they don't need to be changed nearly as often, and then at some future point commit to API stability.

[Steve Dodier-Lazaro (sidi)]

As agreed with Aren, hidden placesholders have been implemented, and the tools menu is entirely ported. Below is the status of plugins:

## Already ported before ##
ABRepeat
Minimode

## Ported ##
IPConsole
Shutdown
Audioscrobbler

## To port ##
Librivox (functionality not broken)
Podcasts (functionality not broken)
Jamendo
Bookmarks
Equalizer
Shoutcast
MultiAlarmClock
ContextInfo
DaapClient

[reacocard (reacocard)]

A couple things:
- There's some noise about shutdown plugin icons, which should be a separate patch (and bug).
- In randomize_playlist_cb and track_properties_cb, you should use get_selected_playlist() instead of get_main().get_selected_page(), since it takes into account the possibility of pages that are not playlists as well. It returns None if the current page is not a playlist page.

Other than that it looks good and appears to work perfectly.

[Dustin Spicuzza (dustin-virtualroadside)]

This looks like it has already been partially implemented. What should we do with the patch?

[Mathias Brodala (mathbr)]

Most of the stuff is done, the only parts which have not yet been applied are the changes to xlgui.__init__ and xlgui.main. These need to be checked thoroughly.

[Dustin Spicuzza (dustin-virtualroadside)]

I'm calling this done. Oldmenu has been removed in r4447. Any plugins that aren't ported should be reported as a specific bug, but I think most of it is done at this point.