PluginAPI
There are a number of problems with beets' plugin API that need to be addressed in a refactoring release. Implementation is currently in progress in an issue.
It's very confusing that so many aspects of plugins happen at the level of the class itself. It would be much more natural for plugins to be treated as singletons, using self to store data.
A couple of plugins (e.g., ihate) actually implement singleton-like behavior: they ensure that only a single instance of the class is created and then use that object for all their work.
We should essentially enshrine this behavior: get rid of all class methods and class-level attributes and replace them with instance-level behaviors.
This dovetails well with the unification of events and data-collection (event handlers will be implemented as methods). See the next topic.
For convenience/cleanliness, we can use Trac's trick to force singleton usage -- __init__ called twice returns an existing instance.
class BeetsPlugin(object):
_instance = None
def __new__(cls):
if cls._instance is None:
cls._instance = type.__new__(cls)
return _instance
plugin_classes = []
...
for obj in mod.__dict__.values():
if BeetsPlugin in obj.__mro__:
plugin_classes.append(obj)
plugin_instances = [cls() for cls in plugin_classes]
One writeup on about this kind of approach: http://martyalchin.com/2008/jan/10/simple-plugin-framework/
There is no intrinsic reason for plugins to be singletons. We rather want a single instance of a plugin per plugin registry. It is therefore better to enforce the singleton behaviour in the plugin registry itself.
The current plugin API suffers from a lot of asymmetry. Some aspects are special methods on the BeetsPlugin class; some aspects are event handlers and need registration via a decorator or explicit call; some are attributes set on the object in its __init__ method. This leads to a confusing interface for developers and unnecessarily complicated logic on our end.
We should make everything into a method on BeetsPlugin. Core beets will invoke these methods with loose binding, akin to the current event system, like plugins.send('foo'). The plugins.py logic will introspect each plugin object to see whether it has the foo method; if so, it calls this method. Crucially, the results from all of these calls are gathered together and returned to the call in core, so send('foo') returns a list of responses to the foo event.
Now, for example, a plugin can just write def pluginload(self): to respond to the pluginload event instead of needing to explicitly register the handler. The commands method used by many plugins stays the same.
The method approach might however lead to boilerplate code when multiple functions use one plugin hook. Consider for example a plugin that wants to add two template functions. This would require the following implementation
class MyPlugin(BeetsPlugin):
def template_functions(self):
return {
'a': self.template_function_a,
'b': self.template_function_b
}
def template_function_a(self):
pass
def template_function_b(self):
passNaturally we would like to use decorators for the template functions to make this more idiomatic. Unfortunately they are not powerful enough to solve this. We need to resort to some hackery.
def template_function(func):
func.__template_function = True
return func
class BeetsPlugin(object):
def template_functions(self):
functions = {}
for method in self.__class__.__dict__.values():
if hasattr(method, '__template_function'):
name = method.__name__
functions[name] = getattr(self, name)
return functionsLots of plugins have very similar logic. We can help simplify these plugins, make them more full-featured, and avoid common bugs by providing subclasses of BeetsPlugin that provide common functionality.
First, a good portion of our plugins are metadata fetchers (fetchart, lyrics, lastgenre, echonest, replaygain, ...). These plugins all need to:
- Add an import hook to run automatically
- Provide an
autooption to disable the import hook - Provide a command for fetching explicitly, which takes a query and has a few standard options (
--force,--nowrite, etc.)
There's no reason for every plugin to re-implement this boilerplate.
Similarly, we should also provide a base class for matching sources (e.g., discogs and amazon).
Stop using a namespace package for plugins. This is causing headaches because [pip-installed packages have problems with namespace packages][pipbug]. [Flask has moved away from a flaskext package][flaskfix], so it might be wise to use Armin's example there. Plugins should be called beets_X or, for plugins distributed as part of beets, beets.plugin.X.
__import__('beets.plug.{}'.format(...)) # built-in
modname = 'beets_{}'.format(...)
import imp
imp.find_module(modname, pluginpaths) # on path
imp.find_module(modname) # installed in Python tree
We could eventually move away from optparse. In particular, Argh is a really clean-looking wrapper for the newer argparse. To use it, however, we'll need to do something horrible to monkey-patch 2.7's argparse to support aliases. I wrote the patch that adds alias support in 3.2, but it is not backported to 2.7: http://hg.python.org/cpython/rev/4c0426261148/
Beets has a mechanism to add new tags to MediaFile in plugins. On the original PR for the plugin architecture refactor @sampsyo said that this should be removed. In #2621 it was discussed that it globally modifies MediaFile, which is undesirable.
A search on GitHub shows several plugins using it (example) so we shouldn't just rip it out. It would be nice to implement it on top of the custom field <-> tag mapping feature.