From a15dffad1d3ed1881acd691df9a050ae6088d5a9 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 11 Dec 2015 15:58:52 +0100 Subject: [PATCH 1/8] Add "before_choose_candidate" event * Add "before_choose_candidate" event, sent to the plugins during TerminalImportSession.choose_match(). This event allows plugins to append additional choice to the user prompt during importer. * The event is sent on the main loop of choose_match(), with session and task parameters. The plugins are responsible for checking the task status in order to append choices in a sensible way. * Add "ExtraChoice" namedtuple to improve readability and encapsulate the concept of a single extra choice. --- beets/ui/commands.py | 47 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 2d943eee1..5d6a556ce 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -22,6 +22,9 @@ from __future__ import (division, absolute_import, print_function, import os import re +from collections import namedtuple +from itertools import chain +from string import lowercase, whitespace import beets from beets import ui @@ -39,6 +42,7 @@ from beets import logging from beets.util.confit import _package_path VARIOUS_ARTISTS = u'Various Artists' +ExtraChoice = namedtuple('ExtraChoice', ['plugin', 'id', 'long', 'callback']) # Global logger. log = logging.getLogger('beets') @@ -471,7 +475,8 @@ def _summary_judment(rec): def choose_candidate(candidates, singleton, rec, cur_artist=None, - cur_album=None, item=None, itemcount=None): + cur_album=None, item=None, itemcount=None, + extra_choices=[]): """Given a sorted list of candidates, ask the user for a selection of which candidate to use. Applies to both full albums and singletons (tracks). Candidates are either AlbumMatch or TrackMatch @@ -489,6 +494,13 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, assert cur_artist is not None assert cur_album is not None + # Build helper variables for extra choices. + # TODO: move "abort" choice always to the end? + # TODO: add plugin name info on choices ("(mb) Print tracks")? + extra_opts = tuple([c.long for c in extra_choices]) + extra_actions = {c.long.strip(lowercase + whitespace)[0].lower(): c.id + for c in extra_choices} + # Zero candidates. if not candidates: if singleton: @@ -502,7 +514,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, 'http://beets.readthedocs.org/en/latest/faq.html#nomatch') opts = ('Use as-is', 'as Tracks', 'Group albums', 'Skip', 'Enter search', 'enter Id', 'aBort') - sel = ui.input_options(opts) + sel = ui.input_options(opts + extra_opts) if sel == 'u': return importer.action.ASIS elif sel == 't': @@ -518,6 +530,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.MANUAL_ID elif sel == 'g': return importer.action.ALBUMS + elif sel in extra_actions.keys(): + return extra_actions[sel] else: assert False @@ -571,7 +585,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, else: opts = ('Skip', 'Use as-is', 'as Tracks', 'Group albums', 'Enter search', 'enter Id', 'aBort') - sel = ui.input_options(opts, numrange=(1, len(candidates))) + sel = ui.input_options(opts + extra_opts, + numrange=(1, len(candidates))) if sel == 's': return importer.action.SKIP elif sel == 'u': @@ -589,6 +604,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.MANUAL_ID elif sel == 'g': return importer.action.ALBUMS + elif sel in extra_actions.keys(): + return extra_actions[sel] else: # Numerical selection. match = candidates[sel - 1] if sel != 1: @@ -623,7 +640,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, }) if default is None: require = True - sel = ui.input_options(opts, require=require, default=default) + sel = ui.input_options(opts + extra_opts, require=require, + default=default) if sel == 'a': return match elif sel == 'g': @@ -641,6 +659,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, raise importer.ImportAbort() elif sel == 'i': return importer.action.MANUAL_ID + elif sel in extra_actions.keys(): + return extra_actions[sel] def manual_search(singleton): @@ -684,10 +704,18 @@ class TerminalImportSession(importer.ImportSession): # Loop until we have a choice. candidates, rec = task.candidates, task.rec while True: + # Gather extra choices from plugins. + # TODO: check for conflicts in choices (duplicated letters, etc) + # TODO: provide a way to override summary_judment and/or non-timid + # mode? Might be useful for some plugins + extra_choices = list(chain(*plugins.send('before_choose_candidate', + session=self, task=task))) + extra_ops = {c.id: c.callback for c in extra_choices} + # Ask for a choice from the user. choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, - itemcount=len(task.items) + itemcount=len(task.items), extra_choices=extra_choices ) # Choose which tags to use. @@ -708,6 +736,15 @@ class TerminalImportSession(importer.ImportSession): _, _, candidates, rec = autotag.tag_album( task.items, search_id=search_id ) + elif choice in extra_ops.keys(): + # Allow extra ops to automatically set the post-choice. + # TODO: currently action.MANUAL and action.MANUAL_ID are not + # properly handled + post_choice = extra_ops[choice](self, task) + if post_choice in (importer.action.SKIP, importer.action.ASIS, + importer.action.TRACKS, + importer.action.ALBUMS): + return post_choice else: # We have a candidate! Finish tagging. Here, choice is an # AlbumMatch object. From 11f4add6873c7c089b5674415276a71b4c03cb42 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 11 Dec 2015 16:04:13 +0100 Subject: [PATCH 2/8] mbsubmit: add example mbsubmit plugin * Add example "mbsubmit" plugin, which allows the user to print the tracklist of an unmatched album during the import process in a format understood by MusicBrainz track parser. * The plugin listens for the before_choose_candidate event and appends the track printing options depending on the task status (candidates and recommendation). --- beetsplug/mbsubmit.py | 68 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 beetsplug/mbsubmit.py diff --git a/beetsplug/mbsubmit.py b/beetsplug/mbsubmit.py new file mode 100644 index 000000000..09be1eb5d --- /dev/null +++ b/beetsplug/mbsubmit.py @@ -0,0 +1,68 @@ +# -*- coding: utf-8 -*- +# This file is part of beets. +# Copyright 2015, Adrian Sampson and Diego Moreda. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Aid in submitting information to MusicBrainz. + +This plugin allows the user to print track information in a format that is +parseable by the MusicBrainz track parser. Programmatic submitting is not +implemented by MusicBrainz yet. +""" + +from __future__ import (division, absolute_import, print_function, + unicode_literals) + + +from beets.autotag import Recommendation +from beets.importer import action +from beets.plugins import BeetsPlugin +from beets.ui.commands import ExtraChoice +from beetsplug.info import print_data + + +class MBSubmitPlugin(BeetsPlugin): + def __init__(self): + super(MBSubmitPlugin, self).__init__() + + self.register_listener('before_choose_candidate', + self.before_choose_candidate_event) + + def before_choose_candidate_event(self, session, task): + # This intends to illustrate a simple plugin that adds choices + # depending on conditions. + # Plugins should return a list of ExtraChoices (basically, the + # "cosmetic" values and a callback function). This list is received and + # flattened on plugins.send('before_choose_candidate'). + if not task.candidates or task.rec == Recommendation.none: + return [ExtraChoice(self, 'PRINT', 'Print tracks', + self.print_tracks), + ExtraChoice(self, 'PRINT_SKIP', 'print tracks and sKip', + self.print_tracks_and_skip)] + + # Callbacks for choices. + def print_tracks(self, session, task): + for i in task.items: + print_data(None, i, '$track. $artist - $title ($length)') + + def print_tracks_and_skip(self, session, task): + # Example of a function that automatically sets the next action, + # avoiding the user to be prompted again. It has some drawbacks (for + # example, actions such as action.MANUAL are not handled properly, as + # they do not exit the main TerminalImportSession.choose_match loop). + # + # The idea is that if a callback function returns an action.X value, + # task.action is set to that value after the callback is processed. + for i in task.items: + print_data(None, i, '$track. $artist - $title ($length)') + return action.SKIP From 70cefb516f46b1aec4cfc6bca4f9a4b629ba8cb0 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 15 Dec 2015 18:33:09 +0100 Subject: [PATCH 3/8] Prompt event style changes and cleanup * Rename "ExtraChoice" to "PromptChoice", and add an attribute for the short letter used by the choice (short). * Add TerminalImportSession._get_plugin_options() for isolating the sending of the event and the processing of the returned values (flattening, check for letter conflicts). * Remove unnecessary TODO lines and other style cleanups. --- beets/ui/commands.py | 71 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 16 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 5d6a556ce..36f0fbf03 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -22,9 +22,8 @@ from __future__ import (division, absolute_import, print_function, import os import re -from collections import namedtuple +from collections import namedtuple, Counter from itertools import chain -from string import lowercase, whitespace import beets from beets import ui @@ -42,7 +41,8 @@ from beets import logging from beets.util.confit import _package_path VARIOUS_ARTISTS = u'Various Artists' -ExtraChoice = namedtuple('ExtraChoice', ['plugin', 'id', 'long', 'callback']) +PromptChoice = namedtuple('ExtraChoice', ['plugin', 'id', 'short', 'long', + 'callback']) # Global logger. log = logging.getLogger('beets') @@ -495,11 +495,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, assert cur_album is not None # Build helper variables for extra choices. - # TODO: move "abort" choice always to the end? - # TODO: add plugin name info on choices ("(mb) Print tracks")? - extra_opts = tuple([c.long for c in extra_choices]) - extra_actions = {c.long.strip(lowercase + whitespace)[0].lower(): c.id - for c in extra_choices} + extra_opts = tuple(c.long for c in extra_choices) + extra_actions = {c.short: c.id for c in extra_choices} # Zero candidates. if not candidates: @@ -705,11 +702,7 @@ class TerminalImportSession(importer.ImportSession): candidates, rec = task.candidates, task.rec while True: # Gather extra choices from plugins. - # TODO: check for conflicts in choices (duplicated letters, etc) - # TODO: provide a way to override summary_judment and/or non-timid - # mode? Might be useful for some plugins - extra_choices = list(chain(*plugins.send('before_choose_candidate', - session=self, task=task))) + extra_choices = self._get_plugin_options(task) extra_ops = {c.id: c.callback for c in extra_choices} # Ask for a choice from the user. @@ -738,10 +731,9 @@ class TerminalImportSession(importer.ImportSession): ) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. - # TODO: currently action.MANUAL and action.MANUAL_ID are not - # properly handled post_choice = extra_ops[choice](self, task) - if post_choice in (importer.action.SKIP, importer.action.ASIS, + if post_choice in (importer.action.SKIP, + importer.action.ASIS, importer.action.TRACKS, importer.action.ALBUMS): return post_choice @@ -838,6 +830,53 @@ class TerminalImportSession(importer.ImportSession): "was interrupted. Resume (Y/n)?" .format(displayable_path(path))) + def _get_plugin_options(self, task): + """Get the extra options appended to the plugins to the ui prompt. + + The `before_choose_candidate` event is sent to the plugins, with + session and task as its parameters. Plugins are responsible for + checking the right conditions and returning a list of `PromptChoice`s, + which is flattened and checked for conflicts. + + Raises `ValueError` if two of the choices have the same short letter. + + Returns a list of `PromptChoice`s. + """ + # Send the before_choose_candidate event and flatten list. + extra_choices = list(chain(*plugins.send('before_choose_candidate', + session=self, task=task))) + # Take into account default options, for duplicate checking. + all_choices = [PromptChoice(self, importer.action.SKIP, 's', 'Skip', + None), + PromptChoice(self, importer.action.ASIS, 'u', + 'Use as-is', None), + PromptChoice(self, importer.action.TRACKS, 't', + 'as Tracks', None), + PromptChoice(self, importer.action.ALBUMS, 'g', + 'Group albums', None), + PromptChoice(self, importer.action.MANUAL, 'e', + 'Enter search', None), + PromptChoice(self, importer.action.MANUAL_ID, 'i', + 'enter Id', None), + PromptChoice(self, '', 'b', 'aBort', None)] +\ + extra_choices + + short_letters = [c.short for c in all_choices] + if len(short_letters) != len(set(short_letters)): + # Duplicate short letter has been found. + duplicates = [i for i, count in Counter(short_letters).items() + if count > 1] + # Build informative message: 'x': classname:"long option",... + dup = {letter: ['%s:"%s"' % (type(c.plugin).__name__, c.long) + for c in all_choices if c.short == letter] + for letter in duplicates} + conflict_msg = '; '.join("'%s': %s" % (k, ', '.join(v)) + for (k, v) in dup.iteritems()) + raise ValueError("Prompt options have the same short letter\n%s" % + conflict_msg) + return extra_choices + + # The import command. From c94679e143268e4da993a248a356fe91ad6c666a Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 15 Dec 2015 20:13:34 +0100 Subject: [PATCH 4/8] Add prompt event documentation * Add documentation for the before_choose_candidate event (add event to the event list and add section with example usage). --- docs/dev/plugins.rst | 67 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 885ef2222..8fa4871bf 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -227,12 +227,17 @@ The events currently available are: of a ``TrackInfo``. Parameter: ``info``. +* `before_choose_candidate`: called before the user is prompted for a decision + during a ``beet import`` interactive session. Plugins can use this event for + :ref:`appending choices to the prompt ` by returning a + list of ``PromptChoices``. Parameters: ``task`` and ``session``. + The included ``mpdupdate`` plugin provides an example use case for event listeners. Extend the Autotagger ^^^^^^^^^^^^^^^^^^^^^ -Plugins in can also enhance the functionality of the autotagger. For a +Plugins can also enhance the functionality of the autotagger. For a comprehensive example, try looking at the ``chroma`` plugin, which is included with beets. @@ -528,3 +533,63 @@ command and an import stage, but the command needs to print more messages than the import stage. (For example, you'll want to log "found lyrics for this song" when you're run explicitly as a command, but you don't want to noisily interrupt the importer interface when running automatically.) + +.. _append_prompt_choices: + +Append Prompt Choices +^^^^^^^^^^^^^^^^^^^^^ + +Plugins can also append choices to the prompt presented to the user during +an import session. + +To do so, add a listener for the ``before_choose_candidate`` event, and return +a list of ``PromptChoices`` that represent the additional choices that your +plugin shall expose to the user:: + + from beets.plugins import BeetsPlugin + + class ExamplePlugin(BeetsPlugin): + def __init__(self): + super(ExamplePlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.before_choose_candidate_event) + + def before_choose_candidate_event(self, session, task): + return [PromptChoice(self, 'PRINT_FOO', 'p', 'Print foo', self.foo), + PromptChoice(self, 'DO_BAR', 'd', 'Do bar', self.bar)] + + def foo(self, session, task): + print('User has chosen "Print foo"!') + + def bar(self, session, task): + print('User has chosen "do Bar"!') + +The previous example modifies the standard prompt:: + + # selection (default 1), Skip, Use as-is, as Tracks, Group albums, + Enter search, enter Id, aBort? + +by appending two additional options (``Print foo`` and ``Do bar``):: + + # selection (default 1), Skip, Use as-is, as Tracks, Group albums, + Enter search, enter Id, aBort, Print foo, Do bar? + +If the user selects a choice, the ``callback`` attribute of the corresponding +``PromptChoice`` will be called. It is the responsibility of the plugin to +check for the status of the import session and decide the choices to be +appended: for example, if a particular choice should only be presented if the +album has no candidates, the relevant checks against ``task.candidates`` should +be performed inside the plugin's ``before_choose_candidate_event`` accordingly. + +Please make sure that the short letter for each of the choices provided by the +plugin is not already in use: the importer will raise an exception if two or +more choices try to use the same short letter. As a reference, the following +characters are used by the choices on the core importer prompt, and hence +should not be used: ``s``, ``u``, ``t``, ``g``, ``e``, ``i``, ``b``. + +Additionally, the callback function can optionally specify the next action to +be performed by returning one of the values from ``importer.action``, which +will be passed to the main loop upon the callback has been processed. Note that +``action.MANUAL`` and ``action.MANUAL_ID`` will have no effect even if +returned by the callback, due to the current arquitecture of the import +process. From f5241f7c28d9547b3ffb2265ec795bc777f6b6c1 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 15 Dec 2015 20:15:42 +0100 Subject: [PATCH 5/8] mbsubmit: update to style changes, remove comments * Update mbsubmit plugin in order to use PromptChoice instead of the previous ExtraChoice. * Remove comments as they are now present on the pull request and documentation. --- beetsplug/mbsubmit.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/beetsplug/mbsubmit.py b/beetsplug/mbsubmit.py index 09be1eb5d..5e839b626 100644 --- a/beetsplug/mbsubmit.py +++ b/beetsplug/mbsubmit.py @@ -27,7 +27,7 @@ from __future__ import (division, absolute_import, print_function, from beets.autotag import Recommendation from beets.importer import action from beets.plugins import BeetsPlugin -from beets.ui.commands import ExtraChoice +from beets.ui.commands import PromptChoice from beetsplug.info import print_data @@ -39,16 +39,12 @@ class MBSubmitPlugin(BeetsPlugin): self.before_choose_candidate_event) def before_choose_candidate_event(self, session, task): - # This intends to illustrate a simple plugin that adds choices - # depending on conditions. - # Plugins should return a list of ExtraChoices (basically, the - # "cosmetic" values and a callback function). This list is received and - # flattened on plugins.send('before_choose_candidate'). if not task.candidates or task.rec == Recommendation.none: - return [ExtraChoice(self, 'PRINT', 'Print tracks', - self.print_tracks), - ExtraChoice(self, 'PRINT_SKIP', 'print tracks and sKip', - self.print_tracks_and_skip)] + return [PromptChoice(self, 'PRINT', 'p', 'Print tracks', + self.print_tracks), + PromptChoice(self, 'PRINT_SKIP', 'k', + 'print tracks and sKip', + self.print_tracks_and_skip)] # Callbacks for choices. def print_tracks(self, session, task): @@ -56,13 +52,6 @@ class MBSubmitPlugin(BeetsPlugin): print_data(None, i, '$track. $artist - $title ($length)') def print_tracks_and_skip(self, session, task): - # Example of a function that automatically sets the next action, - # avoiding the user to be prompted again. It has some drawbacks (for - # example, actions such as action.MANUAL are not handled properly, as - # they do not exit the main TerminalImportSession.choose_match loop). - # - # The idea is that if a callback function returns an action.X value, - # task.action is set to that value after the callback is processed. for i in task.items: print_data(None, i, '$track. $artist - $title ($length)') return action.SKIP From fc08b4665d71d9be3933024ef476670afa3b7d0a Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 16 Dec 2015 19:10:30 +0100 Subject: [PATCH 6/8] Prompt event cleanup, conflict solving, singleton * Simplify PromptChoice so "plugin" and "id" fields are removed, updating the loops and the rest of the code to reflect this change. * Solve short letter conflicts by keeping one of the choices and removing the rest, instead of by raising an Exception. * Misc cleanups as suggested on #1758 discussion. --- beets/ui/commands.py | 92 ++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 36f0fbf03..4f87ad84e 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -41,8 +41,7 @@ from beets import logging from beets.util.confit import _package_path VARIOUS_ARTISTS = u'Various Artists' -PromptChoice = namedtuple('ExtraChoice', ['plugin', 'id', 'short', 'long', - 'callback']) +PromptChoice = namedtuple('ExtraChoice', ['short', 'long', 'callback']) # Global logger. log = logging.getLogger('beets') @@ -484,8 +483,16 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, `cur_album`, and `itemcount` must be provided. For singletons, `item` must be provided. - Returns the result of the choice, which may SKIP, ASIS, TRACKS, or - MANUAL or a candidate (an AlbumMatch/TrackMatch object). + `extra_choices` is a list of `PromptChoice`s, containg the choices + appended by the plugins after receiving the `before_choose_candidate` + event. If not empty, the choices are appended to the prompt presented + to the user. + + Returns one of the following: + * the result of the choice, which may be SKIP, ASIS, TRACKS, or MANUAL + * a candidate (an AlbumMatch/TrackMatch object) + * the short letter of a `PromptChoice` (if the user selected one of + the `extra_choices`). """ # Sanity check. if singleton: @@ -496,7 +503,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Build helper variables for extra choices. extra_opts = tuple(c.long for c in extra_choices) - extra_actions = {c.short: c.id for c in extra_choices} + extra_actions = tuple(c.short for c in extra_choices) # Zero candidates. if not candidates: @@ -527,8 +534,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.MANUAL_ID elif sel == 'g': return importer.action.ALBUMS - elif sel in extra_actions.keys(): - return extra_actions[sel] + elif sel in extra_actions: + return sel else: assert False @@ -601,8 +608,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.MANUAL_ID elif sel == 'g': return importer.action.ALBUMS - elif sel in extra_actions.keys(): - return extra_actions[sel] + elif sel in extra_actions: + return sel else: # Numerical selection. match = candidates[sel - 1] if sel != 1: @@ -656,8 +663,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, raise importer.ImportAbort() elif sel == 'i': return importer.action.MANUAL_ID - elif sel in extra_actions.keys(): - return extra_actions[sel] + elif sel in extra_actions: + return sel def manual_search(singleton): @@ -702,8 +709,8 @@ class TerminalImportSession(importer.ImportSession): candidates, rec = task.candidates, task.rec while True: # Gather extra choices from plugins. - extra_choices = self._get_plugin_options(task) - extra_ops = {c.id: c.callback for c in extra_choices} + extra_choices = self._get_plugin_choices(task) + extra_ops = {c.short: c.callback for c in extra_choices} # Ask for a choice from the user. choice = choose_candidate( @@ -732,10 +739,8 @@ class TerminalImportSession(importer.ImportSession): elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. post_choice = extra_ops[choice](self, task) - if post_choice in (importer.action.SKIP, - importer.action.ASIS, - importer.action.TRACKS, - importer.action.ALBUMS): + if isinstance(post_choice, importer.action): + # MANUAL and MANUAL_ID have no effect, even if returned. return post_choice else: # We have a candidate! Finish tagging. Here, choice is an @@ -761,8 +766,12 @@ class TerminalImportSession(importer.ImportSession): return action while True: + extra_choices = self._get_plugin_choices(task) + extra_ops = {c.short: c.callback for c in extra_choices} + # Ask for a choice. - choice = choose_candidate(candidates, True, rec, item=task.item) + choice = choose_candidate(candidates, True, rec, item=task.item, + extra_choices=extra_choices) if choice in (importer.action.SKIP, importer.action.ASIS): return choice @@ -779,6 +788,12 @@ class TerminalImportSession(importer.ImportSession): if search_id: candidates, rec = autotag.tag_item(task.item, search_id=search_id) + elif choice in extra_ops.keys(): + # Allow extra ops to automatically set the post-choice. + post_choice = extra_ops[choice](self, task) + if isinstance(post_choice, importer.action): + # MANUAL and MANUAL_ID have no effect, even if returned. + return post_choice else: # Chose a candidate. assert isinstance(choice, autotag.TrackMatch) @@ -830,8 +845,8 @@ class TerminalImportSession(importer.ImportSession): "was interrupted. Resume (Y/n)?" .format(displayable_path(path))) - def _get_plugin_options(self, task): - """Get the extra options appended to the plugins to the ui prompt. + def _get_plugin_choices(self, task): + """Get the extra choices appended to the plugins to the ui prompt. The `before_choose_candidate` event is sent to the plugins, with session and task as its parameters. Plugins are responsible for @@ -846,19 +861,14 @@ class TerminalImportSession(importer.ImportSession): extra_choices = list(chain(*plugins.send('before_choose_candidate', session=self, task=task))) # Take into account default options, for duplicate checking. - all_choices = [PromptChoice(self, importer.action.SKIP, 's', 'Skip', - None), - PromptChoice(self, importer.action.ASIS, 'u', - 'Use as-is', None), - PromptChoice(self, importer.action.TRACKS, 't', - 'as Tracks', None), - PromptChoice(self, importer.action.ALBUMS, 'g', - 'Group albums', None), - PromptChoice(self, importer.action.MANUAL, 'e', - 'Enter search', None), - PromptChoice(self, importer.action.MANUAL_ID, 'i', - 'enter Id', None), - PromptChoice(self, '', 'b', 'aBort', None)] +\ + all_choices = [PromptChoice('a', 'Apply', None), + PromptChoice('s', 'Skip', None), + PromptChoice('u', 'Use as-is', None), + PromptChoice('t', 'as Tracks', None), + PromptChoice('g', 'Group albums', None), + PromptChoice('e', 'Enter search', None), + PromptChoice('i', 'enter Id', None), + PromptChoice('b', 'aBort', None)] +\ extra_choices short_letters = [c.short for c in all_choices] @@ -866,14 +876,14 @@ class TerminalImportSession(importer.ImportSession): # Duplicate short letter has been found. duplicates = [i for i, count in Counter(short_letters).items() if count > 1] - # Build informative message: 'x': classname:"long option",... - dup = {letter: ['%s:"%s"' % (type(c.plugin).__name__, c.long) - for c in all_choices if c.short == letter] - for letter in duplicates} - conflict_msg = '; '.join("'%s': %s" % (k, ', '.join(v)) - for (k, v) in dup.iteritems()) - raise ValueError("Prompt options have the same short letter\n%s" % - conflict_msg) + for short in duplicates: + # Keep the first of the choices, removing the rest. + dup_choices = [c for c in all_choices if c.short == short] + for c in dup_choices[1:]: + log.warn(u"Prompt choice '{0}' removed due to conflict " + u"with '{1}' (short letter: '{2}')", + c.long, dup_choices[0].long, c.short) + extra_choices.remove(c) return extra_choices From b7747013d34825d4c5df3961c445ebd56452bd7e Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 16 Dec 2015 19:23:27 +0100 Subject: [PATCH 7/8] Prompt event unit tests * Add "before_choose_candidate" unit tests (PromptChoicesTest), containing tests for checking the addition of choices to ui.input_options (album and singletons), conflict resolution, and callback handling (regular and with return value). --- test/test_plugins.py | 158 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 4 deletions(-) diff --git a/test/test_plugins.py b/test/test_plugins.py index 8e6689c6d..95d3c8fb8 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -17,17 +17,18 @@ from __future__ import (division, absolute_import, print_function, unicode_literals) import os -from mock import patch, Mock +from mock import patch, Mock, ANY import shutil import itertools from beets.importer import SingletonImportTask, SentinelImportTask, \ - ArchiveImportTask -from beets import plugins, config + ArchiveImportTask, action +from beets import plugins, config, ui from beets.library import Item from beets.dbcore import types from beets.mediafile import MediaFile -from test.test_importer import ImportHelper +from test.test_importer import ImportHelper, AutotagStub +from test.test_ui_importer import TerminalImportSessionSetup from test._common import unittest, RSRC from test import helper @@ -401,6 +402,155 @@ class ListenersTest(unittest.TestCase, TestHelper): plugins.send('event9', foo=5) +class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, + ImportHelper, TestHelper): + def setUp(self): + self.setup_plugin_loader() + self.setup_beets() + self._create_import_dir(3) + self._setup_import_session() + self.matcher = AutotagStub().install() + # keep track of ui.input_option() calls + self.input_options_patcher = patch('beets.ui.input_options', + side_effect=ui.input_options) + self.mock_input_options = self.input_options_patcher.start() + + def tearDown(self): + self.input_options_patcher.stop() + self.teardown_plugin_loader() + self.teardown_beets() + + def test_plugin_choices_in_ui_input_options_album(self): + """Test the presence of plugin choices on the prompt (album).""" + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.return_choices) + + def return_choices(self, session, task): + return [ui.commands.PromptChoice('f', 'Foo', None), + ui.commands.PromptChoice('r', 'baR', None)] + + self.register_plugin(DummyPlugin) + # Default options + extra choices by the plugin ('Foo', 'Bar') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + ('Foo', 'baR') + + self.importer.add_choice(action.SKIP) + self.importer.run() + self.mock_input_options.assert_called_once_with(opts, default='a', + require=ANY) + + def test_plugin_choices_in_ui_input_options_singleton(self): + """Test the presence of plugin choices on the prompt (singleton).""" + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.return_choices) + + def return_choices(self, session, task): + return [ui.commands.PromptChoice('f', 'Foo', None), + ui.commands.PromptChoice('r', 'baR', None)] + + self.register_plugin(DummyPlugin) + # Default options + extra choices by the plugin ('Foo', 'Bar') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', + u'Enter search', + u'enter Id', u'aBort') + ('Foo', 'baR') + + config['import']['singletons'] = True + self.importer.add_choice(action.SKIP) + self.importer.run() + self.mock_input_options.assert_called_with(opts, default='a', + require=ANY) + + def test_choices_conflicts(self): + """Test the short letter conflict solving.""" + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.return_choices) + + def return_choices(self, session, task): + return [ui.commands.PromptChoice('a', 'A foo', None), # dupe + ui.commands.PromptChoice('z', 'baZ', None), # ok + ui.commands.PromptChoice('z', 'Zupe', None), # dupe + ui.commands.PromptChoice('z', 'Zoo', None)] # dupe + + self.register_plugin(DummyPlugin) + # Default options + not dupe extra choices by the plugin ('baZ') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + ('baZ',) + self.importer.add_choice(action.SKIP) + self.importer.run() + self.mock_input_options.assert_called_once_with(opts, default='a', + require=ANY) + + def test_plugin_callback(self): + """Test that plugin callbacks are being called upon user choice.""" + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.return_choices) + + def return_choices(self, session, task): + return [ui.commands.PromptChoice('f', 'Foo', self.foo)] + + def foo(self, session, task): + pass + + self.register_plugin(DummyPlugin) + # Default options + extra choices by the plugin ('Foo', 'Bar') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + ('Foo',) + + # DummyPlugin.foo() should be called once + with patch.object(DummyPlugin, 'foo', autospec=True) as mock_foo: + with helper.control_stdin('\n'.join(['f', 's'])): + self.importer.run() + self.assertEqual(mock_foo.call_count, 1) + + # input_options should be called twice, as foo() returns None + self.assertEqual(self.mock_input_options.call_count, 2) + self.mock_input_options.assert_called_with(opts, default='a', + require=ANY) + + def test_plugin_callback_return(self): + """Test that plugin callbacks that return a value exit the loop.""" + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + super(DummyPlugin, self).__init__() + self.register_listener('before_choose_candidate', + self.return_choices) + + def return_choices(self, session, task): + return [ui.commands.PromptChoice('f', 'Foo', self.foo)] + + def foo(self, session, task): + return action.SKIP + + self.register_plugin(DummyPlugin) + # Default options + extra choices by the plugin ('Foo', 'Bar') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + ('Foo',) + + # DummyPlugin.foo() should be called once + with helper.control_stdin('f\n'): + self.importer.run() + + # input_options should be called once, as foo() returns SKIP + self.mock_input_options.assert_called_once_with(opts, default='a', + require=ANY) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 6f8a1aa40a7d79bf849a77a2cd12b0e54addddb0 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 16 Dec 2015 19:26:39 +0100 Subject: [PATCH 8/8] Prompt event documentation and mbsubmit fixes * Fixes for documentation of the prompt event (missing reserved letter, typos). * Update sample mbsubmit plugin to conform to the new PromptChoice structure. --- beetsplug/mbsubmit.py | 6 ++---- docs/dev/plugins.rst | 11 ++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/beetsplug/mbsubmit.py b/beetsplug/mbsubmit.py index 5e839b626..e86ca1ab7 100644 --- a/beetsplug/mbsubmit.py +++ b/beetsplug/mbsubmit.py @@ -40,10 +40,8 @@ class MBSubmitPlugin(BeetsPlugin): def before_choose_candidate_event(self, session, task): if not task.candidates or task.rec == Recommendation.none: - return [PromptChoice(self, 'PRINT', 'p', 'Print tracks', - self.print_tracks), - PromptChoice(self, 'PRINT_SKIP', 'k', - 'print tracks and sKip', + return [PromptChoice('p', 'Print tracks', self.print_tracks), + PromptChoice('k', 'print tracks and sKip', self.print_tracks_and_skip)] # Callbacks for choices. diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 8fa4871bf..3b3ad80a2 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -547,6 +547,7 @@ a list of ``PromptChoices`` that represent the additional choices that your plugin shall expose to the user:: from beets.plugins import BeetsPlugin + from beets.ui.commands import PromptChoice class ExamplePlugin(BeetsPlugin): def __init__(self): @@ -555,14 +556,14 @@ plugin shall expose to the user:: self.before_choose_candidate_event) def before_choose_candidate_event(self, session, task): - return [PromptChoice(self, 'PRINT_FOO', 'p', 'Print foo', self.foo), - PromptChoice(self, 'DO_BAR', 'd', 'Do bar', self.bar)] + return [PromptChoice('p', 'Print foo', self.foo), + PromptChoice('d', 'Do bar', self.bar)] def foo(self, session, task): print('User has chosen "Print foo"!') def bar(self, session, task): - print('User has chosen "do Bar"!') + print('User has chosen "Do bar"!') The previous example modifies the standard prompt:: @@ -585,11 +586,11 @@ Please make sure that the short letter for each of the choices provided by the plugin is not already in use: the importer will raise an exception if two or more choices try to use the same short letter. As a reference, the following characters are used by the choices on the core importer prompt, and hence -should not be used: ``s``, ``u``, ``t``, ``g``, ``e``, ``i``, ``b``. +should not be used: ``a``, ``s``, ``u``, ``t``, ``g``, ``e``, ``i``, ``b``. Additionally, the callback function can optionally specify the next action to be performed by returning one of the values from ``importer.action``, which will be passed to the main loop upon the callback has been processed. Note that ``action.MANUAL`` and ``action.MANUAL_ID`` will have no effect even if -returned by the callback, due to the current arquitecture of the import +returned by the callback, due to the current architecture of the import process.