From f93ee3accc6673674cfc98d2a1e09afb119d3eea Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Mon, 26 Dec 2016 23:47:10 +0100 Subject: [PATCH 01/34] First hack of ignoring already tagged items --- beetsplug/acousticbrainz.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 138fd8809..9985ade87 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -155,6 +155,12 @@ class AcousticPlugin(plugins.BeetsPlugin): """Fetch additional information from AcousticBrainz for the `item`s. """ for item in items: + + mood_str = item.get('mood_acoustic', u'') + if len(mood_str) != 0: + self._log.info(u'Already set acousticbrainz tag for {} ', item) + continue + if not item.mb_trackid: continue From bbaad2f17dedf3394065f9bea3eb30fc85df2fae Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Tue, 27 Dec 2016 13:22:16 +0100 Subject: [PATCH 02/34] Introduce force option in acousticbrainz --- beetsplug/acousticbrainz.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 9985ade87..9f5232953 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -107,7 +107,8 @@ class AcousticPlugin(plugins.BeetsPlugin): def __init__(self): super(AcousticPlugin, self).__init__() - self.config.add({'auto': True}) + self.config.add({'auto': True,'force': False}) + if self.config['auto']: self.register_listener('import_task_files', self.import_task_files) @@ -118,7 +119,13 @@ class AcousticPlugin(plugins.BeetsPlugin): def func(lib, opts, args): items = lib.items(ui.decargs(args)) - self._fetch_info(items, ui.should_write()) + self._fetch_info(items, ui.should_write(),opts.force_refetch or self.config['force']) + + cmd.parser.add_option( + u'-f', u'--force', dest='force_refetch', + action='store_true', default=False, + help=u'always fetch acousticbrainz data', + ) cmd.func = func return [cmd] @@ -151,15 +158,16 @@ class AcousticPlugin(plugins.BeetsPlugin): return data - def _fetch_info(self, items, write): + def _fetch_info(self, items, write, force): """Fetch additional information from AcousticBrainz for the `item`s. """ for item in items: - mood_str = item.get('mood_acoustic', u'') - if len(mood_str) != 0: - self._log.info(u'Already set acousticbrainz tag for {} ', item) - continue + if not force: + mood_str = item.get('mood_acoustic', u'') + if len(mood_str) != 0: + self._log.info(u'Already set acousticbrainz tag for {} ', item) + continue if not item.mb_trackid: continue From 8e29a3ffcb3b36fe2baf7ee80c233c9af18a953f Mon Sep 17 00:00:00 2001 From: "nath@home" Date: Tue, 27 Dec 2016 18:46:46 +0100 Subject: [PATCH 03/34] Zero: Last minute unimportant fixes: *Remove the artifact of a debug log. *Remove meaningless version number. *Rephrase some docstrings. *Remove tautological comments. --- beetsplug/zero.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/beetsplug/zero.py b/beetsplug/zero.py index 6a3013069..022c2c721 100644 --- a/beetsplug/zero.py +++ b/beetsplug/zero.py @@ -27,14 +27,12 @@ from beets.ui import Subcommand, decargs, input_yn from beets.util import confit __author__ = 'baobab@heresiarch.info' -__version__ = '0.10' class ZeroPlugin(BeetsPlugin): def __init__(self): super(ZeroPlugin, self).__init__() - # Listeners. self.register_listener('write', self.write_event) self.register_listener('import_task_choice', self.import_task_choice_event) @@ -49,6 +47,13 @@ class ZeroPlugin(BeetsPlugin): self.fields_to_progs = {} self.warned = False + """Read the bulk of the config into `self.fields_to_progs`. + After construction, `fields_to_progs` contains all the fields that + should be zeroed as keys and maps each of those to a list of compiled + regexes (progs) as values. + A field is zeroed if its value matches one of the associated progs. If + progs is empty, then the associated field is always zeroed. + """ if self.config['fields'] and self.config['keep_fields']: self._log.warning( u'cannot blacklist and whitelist at the same time' @@ -80,9 +85,8 @@ class ZeroPlugin(BeetsPlugin): return [zero_command] def _set_pattern(self, field): - """Set a field in `self.patterns` to a string list corresponding to - the configuration, or `True` if the field has no specific - configuration. + """Populate `self.fields_to_progs` for a given field. + Do some sanity checks then compile the regexes. """ if field not in MediaFile.fields(): self._log.error(u'invalid field: {0}', field) @@ -99,20 +103,22 @@ class ZeroPlugin(BeetsPlugin): self.fields_to_progs[field] = [] def import_task_choice_event(self, session, task): - """Listen for import_task_choice event.""" if task.choice_flag == action.ASIS and not self.warned: self._log.warning(u'cannot zero in \"as-is\" mode') self.warned = True # TODO request write in as-is mode def write_event(self, item, path, tags): - """Set values in tags to `None` if the key and value are matched - by `self.patterns`. - """ if self.config['auto']: self.set_fields(item, tags) def set_fields(self, item, tags): + """Set values in `tags` to `None` if the field is in + `self.fields_to_progs` and any of the corresponding `progs` matches the + field value. + Also update the `item` itself if `update_database` is set in the + config. + """ fields_set = False if not self.fields_to_progs: @@ -122,7 +128,7 @@ class ZeroPlugin(BeetsPlugin): for field, progs in self.fields_to_progs.items(): if field in tags: value = tags[field] - match = _match_progs(tags[field], progs, self._log) + match = _match_progs(tags[field], progs) else: value = '' match = not progs @@ -145,9 +151,9 @@ class ZeroPlugin(BeetsPlugin): item.store(fields=tags) -def _match_progs(value, progs, log): - """Check if field (as string) is matching any of the patterns in - the list. +def _match_progs(value, progs): + """Check if `value` (as string) is matching any of the compiled regexes in + the `progs` list. """ if not progs: return True From c632949b648b90b044aa58089a43bfd39f3d6170 Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Tue, 27 Dec 2016 21:48:06 +0100 Subject: [PATCH 04/34] Changes suggested by @Kraymer --- beetsplug/acousticbrainz.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 9f5232953..09078402f 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -119,7 +119,7 @@ class AcousticPlugin(plugins.BeetsPlugin): def func(lib, opts, args): items = lib.items(ui.decargs(args)) - self._fetch_info(items, ui.should_write(),opts.force_refetch or self.config['force']) + self._fetch_info(items, ui.should_write(), opts.force_refetch or self.config['force']) cmd.parser.add_option( u'-f', u'--force', dest='force_refetch', @@ -165,8 +165,8 @@ class AcousticPlugin(plugins.BeetsPlugin): if not force: mood_str = item.get('mood_acoustic', u'') - if len(mood_str) != 0: - self._log.info(u'Already set acousticbrainz tag for {} ', item) + if mood_str: + self._log.info(u'Already set acousticbrainz tags for {} ', item) continue if not item.mb_trackid: From 00371de0bb739cd2f632e57525cbff0db6ea5008 Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Tue, 27 Dec 2016 21:56:39 +0100 Subject: [PATCH 05/34] Changes suggested by @sampsyo --- beetsplug/acousticbrainz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 09078402f..5fc60634d 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -124,7 +124,7 @@ class AcousticPlugin(plugins.BeetsPlugin): cmd.parser.add_option( u'-f', u'--force', dest='force_refetch', action='store_true', default=False, - help=u'always fetch acousticbrainz data', + help=u're-download data when already present' ) cmd.func = func From 8be0a271b599aa0151f0074d36fb904982735b12 Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Tue, 27 Dec 2016 22:11:27 +0100 Subject: [PATCH 06/34] Add documentation for the force-option --- docs/plugins/acousticbrainz.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/plugins/acousticbrainz.rst b/docs/plugins/acousticbrainz.rst index b66bf17de..5d72b8517 100644 --- a/docs/plugins/acousticbrainz.rst +++ b/docs/plugins/acousticbrainz.rst @@ -8,7 +8,12 @@ The ``acousticbrainz`` plugin gets acoustic-analysis information from the Enable the ``acousticbrainz`` plugin in your configuration (see :ref:`using-plugins`) and run it by typing:: - $ beet acousticbrainz [QUERY] + $ beet acousticbrainz [-f] [QUERY] + +By default, the command will only look for acousticbrainz data when the tracks doesn't +already have it; the ``-f`` or ``--force`` switch makes it fetch acousticbrainz +for the item. If you specify a query, only matching tracks will be processed; +otherwise, the command processes every track in your library. For all tracks with a MusicBrainz recording ID, the plugin currently sets these fields: @@ -52,3 +57,6 @@ configuration file. There is one option: - **auto**: Enable AcousticBrainz during ``beet import``. Default: ``yes``. +- **force**: By default, beets will not override already fetched acousticbrainz data. To instead fetch acousticbrainz and override data, + set the ``force`` option to ``yes``. + Default: ``no``. \ No newline at end of file From 165f2e189ea93c4391b48df710af6ae812e850d4 Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Wed, 28 Dec 2016 15:11:03 +0100 Subject: [PATCH 07/34] Repair findings from Travis CI --- beetsplug/acousticbrainz.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 5fc60634d..3dea02fd2 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -107,7 +107,7 @@ class AcousticPlugin(plugins.BeetsPlugin): def __init__(self): super(AcousticPlugin, self).__init__() - self.config.add({'auto': True,'force': False}) + self.config.add({'auto': True, 'force': False}) if self.config['auto']: self.register_listener('import_task_files', @@ -119,8 +119,8 @@ class AcousticPlugin(plugins.BeetsPlugin): def func(lib, opts, args): items = lib.items(ui.decargs(args)) - self._fetch_info(items, ui.should_write(), opts.force_refetch or self.config['force']) - + self._fetch_info(items, ui.should_write(), + opts.force_refetch or self.config['force']) cmd.parser.add_option( u'-f', u'--force', dest='force_refetch', action='store_true', default=False, @@ -162,11 +162,11 @@ class AcousticPlugin(plugins.BeetsPlugin): """Fetch additional information from AcousticBrainz for the `item`s. """ for item in items: - if not force: mood_str = item.get('mood_acoustic', u'') if mood_str: - self._log.info(u'Already set acousticbrainz tags for {} ', item) + self._log.info(u'Already set acoustic\ + brainz tags for {} ', item) continue if not item.mb_trackid: From 7e1e31bdddea3d30c3a61c0514298bfb1fdbbe6c Mon Sep 17 00:00:00 2001 From: Susanna Maria Hepp Date: Wed, 28 Dec 2016 15:30:57 +0100 Subject: [PATCH 08/34] E128: continuation line under-indented for visual indent? --- beetsplug/acousticbrainz.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 3dea02fd2..725c55089 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -119,8 +119,8 @@ class AcousticPlugin(plugins.BeetsPlugin): def func(lib, opts, args): items = lib.items(ui.decargs(args)) - self._fetch_info(items, ui.should_write(), - opts.force_refetch or self.config['force']) + self._fetch_info(items, ui.should_write(), + opts.force_refetch or self.config['force']) cmd.parser.add_option( u'-f', u'--force', dest='force_refetch', action='store_true', default=False, From c27879edbc5d5a3dacc09fc7f853a29cbcd8dacc Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 11:59:14 -0500 Subject: [PATCH 09/34] Slight code formatting tweaks for #2349 --- beetsplug/acousticbrainz.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 725c55089..93647170b 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -107,7 +107,10 @@ class AcousticPlugin(plugins.BeetsPlugin): def __init__(self): super(AcousticPlugin, self).__init__() - self.config.add({'auto': True, 'force': False}) + self.config.add({ + 'auto': True, + 'force': False, + }) if self.config['auto']: self.register_listener('import_task_files', @@ -116,17 +119,17 @@ class AcousticPlugin(plugins.BeetsPlugin): def commands(self): cmd = ui.Subcommand('acousticbrainz', help=u"fetch metadata from AcousticBrainz") - - def func(lib, opts, args): - items = lib.items(ui.decargs(args)) - self._fetch_info(items, ui.should_write(), - opts.force_refetch or self.config['force']) cmd.parser.add_option( u'-f', u'--force', dest='force_refetch', action='store_true', default=False, help=u're-download data when already present' ) + def func(lib, opts, args): + items = lib.items(ui.decargs(args)) + self._fetch_info(items, ui.should_write(), + opts.force_refetch or self.config['force']) + cmd.func = func return [cmd] @@ -162,13 +165,17 @@ class AcousticPlugin(plugins.BeetsPlugin): """Fetch additional information from AcousticBrainz for the `item`s. """ for item in items: + # If we're not forcing re-downloading for all tracks, check + # whether the data is already present. We use one + # representative field name to check for previously fetched + # data. if not force: mood_str = item.get('mood_acoustic', u'') if mood_str: - self._log.info(u'Already set acoustic\ - brainz tags for {} ', item) + self._log.info(u'data already present for: {}', item) continue + # We can only fetch data for tracks with MBIDs. if not item.mb_trackid: continue From 2a3f3d9bfa9015dbddc838342d4c6f091d7d7d92 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 12:01:08 -0500 Subject: [PATCH 10/34] acousticbrainz: Make some strings into comments When not in the docstring position, it's better to use "real" comments instead of string literals. --- beetsplug/acousticbrainz.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 93647170b..2e99637b3 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -212,7 +212,8 @@ class AcousticPlugin(plugins.BeetsPlugin): joined with `' '`. This is hardcoded and not very flexible, but it gets the job done. - Example: + For example: + >>> scheme = { 'key1': 'attribute', 'key group': { @@ -234,24 +235,24 @@ class AcousticPlugin(plugins.BeetsPlugin): ('attribute', 'value'), ('composite attribute', 'part 1 of composite attr part 2')] """ - """First, we traverse `scheme` and `data`, `yield`ing all the non - composites attributes straight away and populating the dictionary - `composites` with the composite attributes. + # First, we traverse `scheme` and `data`, `yield`ing all the non + # composites attributes straight away and populating the dictionary + # `composites` with the composite attributes. - When we are finished traversing `scheme`, `composites` should map - each composite attribute to an ordered list of the values belonging to - the attribute, for example: - `composites = {'initial_key': ['B', 'minor']}`. - """ + # When we are finished traversing `scheme`, `composites` should map + # each composite attribute to an ordered list of the values belonging to + # the attribute, for example: + # `composites = {'initial_key': ['B', 'minor']}`. + + # The recursive traversal. composites = defaultdict(list) - # The recursive traversal for attr, val in self._data_to_scheme_child(data, scheme, composites): yield attr, val - """When composites has been populated, yield the composite attributes - by joining their parts. - """ + + # When composites has been populated, yield the composite attributes + # by joining their parts. for composite_attr, value_parts in composites.items(): yield composite_attr, ' '.join(value_parts) From 8a62087376ce27cbb694298599ec4f76b4cf9d3e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 12:03:57 -0500 Subject: [PATCH 11/34] Documentation tweaks for #2349 --- beetsplug/acousticbrainz.py | 6 +++--- docs/plugins/acousticbrainz.rst | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index 2e99637b3..a323ce681 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -239,9 +239,9 @@ class AcousticPlugin(plugins.BeetsPlugin): # composites attributes straight away and populating the dictionary # `composites` with the composite attributes. - # When we are finished traversing `scheme`, `composites` should map - # each composite attribute to an ordered list of the values belonging to - # the attribute, for example: + # When we are finished traversing `scheme`, `composites` should + # map each composite attribute to an ordered list of the values + # belonging to the attribute, for example: # `composites = {'initial_key': ['B', 'minor']}`. # The recursive traversal. diff --git a/docs/plugins/acousticbrainz.rst b/docs/plugins/acousticbrainz.rst index 5d72b8517..de4c667c0 100644 --- a/docs/plugins/acousticbrainz.rst +++ b/docs/plugins/acousticbrainz.rst @@ -10,10 +10,11 @@ Enable the ``acousticbrainz`` plugin in your configuration (see :ref:`using-plug $ beet acousticbrainz [-f] [QUERY] -By default, the command will only look for acousticbrainz data when the tracks doesn't -already have it; the ``-f`` or ``--force`` switch makes it fetch acousticbrainz -for the item. If you specify a query, only matching tracks will be processed; -otherwise, the command processes every track in your library. +By default, the command will only look for AcousticBrainz data when the tracks +doesn't already have it; the ``-f`` or ``--force`` switch makes it re-download +data even when it already exists. If you specify a query, only matching tracks +will be processed; otherwise, the command processes every track in your +library. For all tracks with a MusicBrainz recording ID, the plugin currently sets these fields: @@ -57,6 +58,6 @@ configuration file. There is one option: - **auto**: Enable AcousticBrainz during ``beet import``. Default: ``yes``. -- **force**: By default, beets will not override already fetched acousticbrainz data. To instead fetch acousticbrainz and override data, - set the ``force`` option to ``yes``. - Default: ``no``. \ No newline at end of file +- **force**: Download AcousticBrainz data even for tracks that already have + it. + Default: ``no``. From d447d3e6552780759944910a7c653bfe6f63333c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 12:06:29 -0500 Subject: [PATCH 12/34] Changelog note for #2349 (fix #2347) --- docs/changelog.rst | 4 ++++ docs/plugins/acousticbrainz.rst | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 43c651200..f3124271c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,10 @@ Features: :bug:`2305` :bug:`2322` * :doc:`/plugins/zero`: Added ``zero`` command to manually trigger the zero plugin. Thanks to :user:`SJoshBrown`. :bug:`2274` :bug:`2329` +* :doc:`/plugins/acousticbrainz`: The plugin will avoid re-downloading data + for files that already have it by default. You can override this behavior + using a new ``force`` option. Thanks to :user:`SusannaMaria`. :bug:`2347` + :bug:`2349` Fixes: diff --git a/docs/plugins/acousticbrainz.rst b/docs/plugins/acousticbrainz.rst index de4c667c0..bf2102790 100644 --- a/docs/plugins/acousticbrainz.rst +++ b/docs/plugins/acousticbrainz.rst @@ -46,7 +46,7 @@ Automatic Tagging ----------------- To automatically tag files using AcousticBrainz data during import, just -enable the ``acousticbrainz`` plugin (see :ref:`using-plugins`). When importing +enable the ``acousticbrainz`` plugin (see :ref:`using-plugins`). When importing new files, beets will query the AcousticBrainz API using MBID and set the appropriate metadata. From 9b0a867c73da102f4e620bbf4faef31be2c018a7 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 12:40:33 -0500 Subject: [PATCH 13/34] Expand some comments in choose_match --- beets/ui/commands.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 8ead3bfad..b9a3e0b91 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -728,27 +728,35 @@ class TerminalImportSession(importer.ImportSession): # Loop until we have a choice. candidates, rec = task.candidates, task.rec while True: - # Gather extra choices from plugins. + # Gather additional menu choices from plugins. We build a + # map from selection characters to callback functions. 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. + # Ask for a choice from the user. The result of + # `choose_candidate` may be an `importer.action` flag or an + # `AlbumMatch` object for a specific selection. choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, itemcount=len(task.items), extra_choices=extra_choices ) - # Choose which tags to use. + # Basic choices that require no more action here. if choice in (importer.action.SKIP, importer.action.ASIS, importer.action.TRACKS, importer.action.ALBUMS): # Pass selection to main control flow. return choice + + # Manual search. We ask for the search terms and run the + # loop again. elif choice is importer.action.MANUAL: # Try again with manual search terms. search_artist, search_album = manual_search(False) _, _, candidates, rec = autotag.tag_album( task.items, search_artist, search_album ) + + # Manual ID. We prompt for the ID and run the loop again. elif choice is importer.action.MANUAL_ID: # Try a manually-entered ID. search_id = manual_id(False) @@ -756,12 +764,16 @@ class TerminalImportSession(importer.ImportSession): _, _, candidates, rec = autotag.tag_album( task.items, search_ids=search_id.split() ) + + # Plugin-provided choices. We invoke the associated callback + # function. elif choice in list(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 + + # Otherwise, we have a specific match selection. else: # We have a candidate! Finish tagging. Here, choice is an # AlbumMatch object. From 3578f0d429d9a3203cd37cdff55312db85e3e433 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 13:21:55 -0500 Subject: [PATCH 14/34] Introduce a new Proposal type for tag results tag_album and tag_item now return a Proposal. The idea is that plugin actions should also be able to return Proposal values, just like the built-in actions. --- beets/autotag/match.py | 32 ++++++++++++++++++++------------ beets/importer.py | 13 ++++++------- beets/ui/commands.py | 24 ++++++++++++++++++------ 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index 493fd20c9..71d80e821 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -22,6 +22,7 @@ from __future__ import division, absolute_import, print_function import datetime import re from munkres import Munkres +from collections import namedtuple from beets import logging from beets import plugins @@ -52,6 +53,13 @@ class Recommendation(OrderedEnum): strong = 3 +# A structure for holding a set of possible matches to choose between. This +# consists of a list of possible candidates (i.e., AlbumInfo or TrackInfo +# objects) and a recommendation value. + +Proposal = namedtuple('Proposal', ('candidates', 'recommendation')) + + # Primary matching functionality. def current_metadata(items): @@ -379,9 +387,8 @@ def _add_candidate(items, results, info): def tag_album(items, search_artist=None, search_album=None, search_ids=[]): - """Return a tuple of a artist name, an album name, a list of - `AlbumMatch` candidates from the metadata backend, and a - `Recommendation`. + """Return a tuple of the current artist name, the current album + name, and a `Proposal` containing `AlbumMatch` candidates. The artist and album are the most common values of these fields among `items`. @@ -429,7 +436,7 @@ def tag_album(items, search_artist=None, search_album=None, if rec == Recommendation.strong: log.debug(u'ID match.') return cur_artist, cur_album, \ - list(candidates.values()), rec + Proposal(list(candidates.values()), rec) # Search terms. if not (search_artist and search_album): @@ -454,14 +461,15 @@ def tag_album(items, search_artist=None, search_album=None, # Sort and get the recommendation. candidates = _sort_candidates(candidates.values()) rec = _recommendation(candidates) - return cur_artist, cur_album, candidates, rec + return cur_artist, cur_album, Proposal(candidates, rec) def tag_item(item, search_artist=None, search_title=None, search_ids=[]): - """Attempts to find metadata for a single track. Returns a - `(candidates, recommendation)` pair where `candidates` is a list of - TrackMatch objects. `search_artist` and `search_title` may be used + """Find metadata for a single track. Return a `Proposal` consisting + of `TrackMatch` objects. + + `search_artist` and `search_title` may be used to override the current metadata for the purposes of the MusicBrainz title. `search_ids` may be used for restricting the search to a list of metadata backend IDs. @@ -484,14 +492,14 @@ def tag_item(item, search_artist=None, search_title=None, if rec == Recommendation.strong and \ not config['import']['timid']: log.debug(u'Track ID match.') - return _sort_candidates(candidates.values()), rec + return Proposal(_sort_candidates(candidates.values()), rec) # If we're searching by ID, don't proceed. if search_ids: if candidates: - return _sort_candidates(candidates.values()), rec + return Proposal(_sort_candidates(candidates.values()), rec) else: - return [], Recommendation.none + return Proposal([], Recommendation.none) # Search terms. if not (search_artist and search_title): @@ -507,4 +515,4 @@ def tag_item(item, search_artist=None, search_title=None, log.debug(u'Found {0} candidates.', len(candidates)) candidates = _sort_candidates(candidates.values()) rec = _recommendation(candidates) - return candidates, rec + return Proposal(candidates, rec) diff --git a/beets/importer.py b/beets/importer.py index 2c1d07c5c..3daf90cdc 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -587,12 +587,12 @@ class ImportTask(BaseImportTask): candidate IDs are stored in self.search_ids: if present, the initial lookup is restricted to only those IDs. """ - artist, album, candidates, recommendation = \ + artist, album, prop = \ autotag.tag_album(self.items, search_ids=self.search_ids) self.cur_artist = artist self.cur_album = album - self.candidates = candidates - self.rec = recommendation + self.candidates = prop.candidates + self.rec = prop.recommendation def find_duplicates(self, lib): """Return a list of albums from `lib` with the same artist and @@ -830,10 +830,9 @@ class SingletonImportTask(ImportTask): plugins.send('item_imported', lib=lib, item=item) def lookup_candidates(self): - candidates, recommendation = autotag.tag_item( - self.item, search_ids=self.search_ids) - self.candidates = candidates - self.rec = recommendation + prop = autotag.tag_item(self.item, search_ids=self.search_ids) + self.candidates = prop.candidates + self.rec = prop.recommendation def find_duplicates(self, lib): """Return a list of items from `lib` that have the same artist diff --git a/beets/ui/commands.py b/beets/ui/commands.py index b9a3e0b91..1a42561fd 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -752,18 +752,22 @@ class TerminalImportSession(importer.ImportSession): elif choice is importer.action.MANUAL: # Try again with manual search terms. search_artist, search_album = manual_search(False) - _, _, candidates, rec = autotag.tag_album( + _, _, prop = autotag.tag_album( task.items, search_artist, search_album ) + candidates = prop.candidates + rec = prop.recommendation # Manual ID. We prompt for the ID and run the loop again. elif choice is importer.action.MANUAL_ID: # Try a manually-entered ID. search_id = manual_id(False) if search_id: - _, _, candidates, rec = autotag.tag_album( + _, _, prop = autotag.tag_album( task.items, search_ids=search_id.split() ) + candidates = prop.candidates + rec = prop.recommendation # Plugin-provided choices. We invoke the associated callback # function. @@ -807,25 +811,33 @@ class TerminalImportSession(importer.ImportSession): if choice in (importer.action.SKIP, importer.action.ASIS): return choice + elif choice == importer.action.TRACKS: assert False # TRACKS is only legal for albums. + elif choice == importer.action.MANUAL: # Continue in the loop with a new set of candidates. search_artist, search_title = manual_search(True) - candidates, rec = autotag.tag_item(task.item, search_artist, - search_title) + prop = autotag.tag_item(task.item, search_artist, search_title) + candidates = prop.candidates + rec = prop.recommendation + elif choice == importer.action.MANUAL_ID: # Ask for a track ID. search_id = manual_id(True) if search_id: - candidates, rec = autotag.tag_item( - task.item, search_ids=search_id.split()) + prop = autotag.tag_item(task.item, + search_ids=search_id.split()) + candidates = prop.candidates + rec = prop.recommendation + elif choice in list(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) From 22610b768408c0abc89a90c15a0348d6f814ab98 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 13:34:27 -0500 Subject: [PATCH 15/34] Refactor extra choice return value The `choose_candidate` function now returns the `PromptChoice` object instead of a short letter that selects the `PromptChoice`. --- beets/ui/commands.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 1a42561fd..1606953b6 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -511,8 +511,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, 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`). + * a chosen `PromptChoice` from `extra_choices` """ # Sanity check. if singleton: @@ -523,7 +522,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 = tuple(c.short for c in extra_choices) + extra_actions = {c.short: c for c in extra_choices} # Zero candidates. if not candidates: @@ -555,7 +554,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: - return sel + return extra_actions[sel] else: assert False @@ -629,7 +628,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: - return sel + return extra_actions[sel] else: # Numerical selection. match = candidates[sel - 1] if sel != 1: @@ -684,7 +683,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u'i': return importer.action.MANUAL_ID elif sel in extra_actions: - return sel + return extra_actions[sel] def manual_search(singleton): @@ -728,14 +727,13 @@ class TerminalImportSession(importer.ImportSession): # Loop until we have a choice. candidates, rec = task.candidates, task.rec while True: - # Gather additional menu choices from plugins. We build a - # map from selection characters to callback functions. + # Gather additional menu choices from plugins. 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. The result of - # `choose_candidate` may be an `importer.action` flag or an - # `AlbumMatch` object for a specific selection. + # `choose_candidate` may be an `importer.action`, an + # `AlbumMatch` object for a specific selection, or a + # `PromptChoice`. choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, itemcount=len(task.items), extra_choices=extra_choices @@ -771,8 +769,8 @@ class TerminalImportSession(importer.ImportSession): # Plugin-provided choices. We invoke the associated callback # function. - elif choice in list(extra_ops.keys()): - post_choice = extra_ops[choice](self, task) + elif choice in extra_choices: + post_choice = choice.callback(self, task) if isinstance(post_choice, importer.action): # MANUAL and MANUAL_ID have no effect, even if returned. return post_choice @@ -803,7 +801,6 @@ class TerminalImportSession(importer.ImportSession): 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, @@ -831,9 +828,8 @@ class TerminalImportSession(importer.ImportSession): candidates = prop.candidates rec = prop.recommendation - elif choice in list(extra_ops.keys()): - # Allow extra ops to automatically set the post-choice. - post_choice = extra_ops[choice](self, task) + elif choice in extra_choices: + post_choice = choice.callback(self, task) if isinstance(post_choice, importer.action): # MANUAL and MANUAL_ID have no effect, even if returned. return post_choice From a414872430960546c8c5d768a73028e0066b11cf Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 13:36:19 -0500 Subject: [PATCH 16/34] Fix a missing parameter (#2349) --- beetsplug/acousticbrainz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/acousticbrainz.py b/beetsplug/acousticbrainz.py index a323ce681..4291d9117 100644 --- a/beetsplug/acousticbrainz.py +++ b/beetsplug/acousticbrainz.py @@ -136,7 +136,7 @@ class AcousticPlugin(plugins.BeetsPlugin): def import_task_files(self, session, task): """Function is called upon beet import. """ - self._fetch_info(task.imported_items(), False) + self._fetch_info(task.imported_items(), False, True) def _get_data(self, mbid): data = {} From 7c6eafa2855b97f8e8ac2f21a9aa6238bb379373 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 13:50:53 -0500 Subject: [PATCH 17/34] Refactor manual search options to use Proposal This is the first step to making them behave like plugin actions. --- beets/ui/commands.py | 65 +++++++++++++++++++++++++------------------- test/test_ui.py | 15 ---------- 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 1606953b6..0d19d7818 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -686,20 +686,40 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return extra_actions[sel] -def manual_search(singleton): - """Input either an artist and album (for full albums) or artist and +def manual_search(task): + """Get a new `Proposal` using manual search criteria. + + Input either an artist and album (for full albums) or artist and track name (for singletons) for manual search. """ - artist = input_(u'Artist:') - name = input_(u'Track:' if singleton else u'Album:') - return artist.strip(), name.strip() + artist = input_(u'Artist:').strip() + name = input_(u'Album:' if task.is_album else u'Track:').strip() + + if task.is_album: + _, _, prop = autotag.tag_album( + task.items, artist, name + ) + return prop + else: + return autotag.tag_item(task.item, artist, name) -def manual_id(singleton): - """Input an ID, either for an album ("release") or a track ("recording"). +def manual_id(task): + """Get a new `Proposal` using a manually-entered ID. + + Input an ID, either for an album ("release") or a track ("recording"). """ - prompt = u'Enter {0} ID:'.format(u'recording' if singleton else u'release') - return input_(prompt).strip() + prompt = u'Enter {0} ID:'.format(u'release' if task.is_album + else u'recording') + search_id = input_(prompt).strip() + + if task.is_album: + _, _, prop = autotag.tag_album( + task.items, search_ids=search_id.split() + ) + return prop + else: + return autotag.tag_item(task.item, search_ids=search_id.split()) class TerminalImportSession(importer.ImportSession): @@ -749,23 +769,16 @@ class TerminalImportSession(importer.ImportSession): # loop again. elif choice is importer.action.MANUAL: # Try again with manual search terms. - search_artist, search_album = manual_search(False) - _, _, prop = autotag.tag_album( - task.items, search_artist, search_album - ) + prop = manual_search(task) candidates = prop.candidates rec = prop.recommendation # Manual ID. We prompt for the ID and run the loop again. elif choice is importer.action.MANUAL_ID: # Try a manually-entered ID. - search_id = manual_id(False) - if search_id: - _, _, prop = autotag.tag_album( - task.items, search_ids=search_id.split() - ) - candidates = prop.candidates - rec = prop.recommendation + prop = manual_id(task) + candidates = prop.candidates + rec = prop.recommendation # Plugin-provided choices. We invoke the associated callback # function. @@ -814,19 +827,15 @@ class TerminalImportSession(importer.ImportSession): elif choice == importer.action.MANUAL: # Continue in the loop with a new set of candidates. - search_artist, search_title = manual_search(True) - prop = autotag.tag_item(task.item, search_artist, search_title) + prop = manual_search(task) candidates = prop.candidates rec = prop.recommendation elif choice == importer.action.MANUAL_ID: # Ask for a track ID. - search_id = manual_id(True) - if search_id: - prop = autotag.tag_item(task.item, - search_ids=search_id.split()) - candidates = prop.candidates - rec = prop.recommendation + prop = manual_id(task) + candidates = prop.candidates + rec = prop.recommendation elif choice in extra_choices: post_choice = choice.callback(self, task) diff --git a/test/test_ui.py b/test/test_ui.py index 39504aa4e..c519e66fb 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -670,21 +670,6 @@ class ImportTest(_common.TestCase): None) -class InputTest(_common.TestCase): - def setUp(self): - super(InputTest, self).setUp() - self.io.install() - - def test_manual_search_gets_unicode(self): - # The input here uses "native strings": bytes on Python 2, Unicode on - # Python 3. - self.io.addinput('foö') - self.io.addinput('bár') - artist, album = commands.manual_search(False) - self.assertEqual(artist, u'foö') - self.assertEqual(album, u'bár') - - @_common.slow_test() class ConfigTest(unittest.TestCase, TestHelper, _common.Assertions): def setUp(self): From 6c825970aca8f0ac026138e3072cec10d3a5cf83 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 13:54:04 -0500 Subject: [PATCH 18/34] Tolerate missing AlbumInfo.mediums field --- beets/ui/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 0d19d7818..f7b32eefe 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -158,7 +158,7 @@ def disambig_string(info): if isinstance(info, hooks.AlbumInfo): if info.media: - if info.mediums > 1: + if info.mediums and info.mediums > 1: disambig.append(u'{0}x{1}'.format( info.mediums, info.media )) From 2f6538aee2ea49fadccf38cd638fe7117b427e43 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:08:09 -0500 Subject: [PATCH 19/34] Refactor search options to ordinary choices `PromptChoice`s can now return `Proposal` values, which makes these two options just "normal" actions that could be provided by plugins. --- beets/autotag/__init__.py | 2 +- beets/importer.py | 4 +- beets/ui/commands.py | 88 +++++++++++++-------------------------- docs/dev/plugins.rst | 5 +-- 4 files changed, 33 insertions(+), 66 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index 09aed89ce..3e79a4498 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -23,7 +23,7 @@ from beets import config # Parts of external interface. from .hooks import AlbumInfo, TrackInfo, AlbumMatch, TrackMatch # noqa -from .match import tag_item, tag_album # noqa +from .match import tag_item, tag_album, Proposal # noqa from .match import Recommendation # noqa # Global logger. diff --git a/beets/importer.py b/beets/importer.py index 3daf90cdc..6a10f4c97 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -43,8 +43,7 @@ from enum import Enum from beets import mediafile action = Enum('action', - ['SKIP', 'ASIS', 'TRACKS', 'MANUAL', 'APPLY', 'MANUAL_ID', - 'ALBUMS', 'RETAG']) + ['SKIP', 'ASIS', 'TRACKS', 'APPLY', 'ALBUMS', 'RETAG']) # The RETAG action represents "don't apply any match, but do record # new metadata". It's not reachable via the standard command prompt but # can be used by plugins. @@ -443,7 +442,6 @@ class ImportTask(BaseImportTask): indicates that an action has been selected for this task. """ # Not part of the task structure: - assert choice not in (action.MANUAL, action.MANUAL_ID) assert choice != action.APPLY # Only used internally. if choice in (action.SKIP, action.ASIS, action.TRACKS, action.ALBUMS, action.RETAG): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index f7b32eefe..82f5da686 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -509,7 +509,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, to the user. Returns one of the following: - * the result of the choice, which may be SKIP, ASIS, TRACKS, or MANUAL + * the result of the choice, which may be SKIP, ASIS, or TRACKS * a candidate (an AlbumMatch/TrackMatch object) * a chosen `PromptChoice` from `extra_choices` """ @@ -536,21 +536,17 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, print_(u'For help, see: ' u'http://beets.readthedocs.org/en/latest/faq.html#nomatch') opts = (u'Use as-is', u'as Tracks', u'Group albums', u'Skip', - u'Enter search', u'enter Id', u'aBort') + u'aBort') sel = ui.input_options(opts + extra_opts) if sel == u'u': return importer.action.ASIS elif sel == u't': assert not singleton return importer.action.TRACKS - elif sel == u'e': - return importer.action.MANUAL elif sel == u's': return importer.action.SKIP elif sel == u'b': raise importer.ImportAbort() - elif sel == u'i': - return importer.action.MANUAL_ID elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: @@ -603,11 +599,10 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Ask the user for a choice. if singleton: - opts = (u'Skip', u'Use as-is', u'Enter search', u'enter Id', - u'aBort') + opts = (u'Skip', u'Use as-is', u'aBort') else: opts = (u'Skip', u'Use as-is', u'as Tracks', u'Group albums', - u'Enter search', u'enter Id', u'aBort') + u'aBort') sel = ui.input_options(opts + extra_opts, numrange=(1, len(candidates))) if sel == u's': @@ -616,15 +611,11 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.ASIS elif sel == u'm': pass - elif sel == u'e': - return importer.action.MANUAL elif sel == u't': assert not singleton return importer.action.TRACKS elif sel == u'b': raise importer.ImportAbort() - elif sel == u'i': - return importer.action.MANUAL_ID elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: @@ -650,11 +641,10 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Ask for confirmation. if singleton: opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', - u'Enter search', u'enter Id', u'aBort') + u'aBort') else: 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') + u'as Tracks', u'Group albums', u'aBort') default = config['import']['default_action'].as_choice({ u'apply': u'a', u'skip': u's', @@ -676,17 +666,13 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u't': assert not singleton return importer.action.TRACKS - elif sel == u'e': - return importer.action.MANUAL elif sel == u'b': raise importer.ImportAbort() - elif sel == u'i': - return importer.action.MANUAL_ID elif sel in extra_actions: return extra_actions[sel] -def manual_search(task): +def manual_search(session, task): """Get a new `Proposal` using manual search criteria. Input either an artist and album (for full albums) or artist and @@ -704,7 +690,7 @@ def manual_search(task): return autotag.tag_item(task.item, artist, name) -def manual_id(task): +def manual_id(session, task): """Get a new `Proposal` using a manually-entered ID. Input an ID, either for an album ("release") or a track ("recording"). @@ -747,8 +733,12 @@ class TerminalImportSession(importer.ImportSession): # Loop until we have a choice. candidates, rec = task.candidates, task.rec while True: - # Gather additional menu choices from plugins. - extra_choices = self._get_plugin_choices(task) + # Set up menu choices. + choices = [ + PromptChoice(u'e', u'Enter search', manual_search), + PromptChoice(u'i', u'enter Id', manual_id), + ] + choices += self._get_plugin_choices(task) # Ask for a choice from the user. The result of # `choose_candidate` may be an `importer.action`, an @@ -756,7 +746,7 @@ class TerminalImportSession(importer.ImportSession): # `PromptChoice`. choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, - itemcount=len(task.items), extra_choices=extra_choices + itemcount=len(task.items), extra_choices=choices ) # Basic choices that require no more action here. @@ -765,28 +755,16 @@ class TerminalImportSession(importer.ImportSession): # Pass selection to main control flow. return choice - # Manual search. We ask for the search terms and run the - # loop again. - elif choice is importer.action.MANUAL: - # Try again with manual search terms. - prop = manual_search(task) - candidates = prop.candidates - rec = prop.recommendation - - # Manual ID. We prompt for the ID and run the loop again. - elif choice is importer.action.MANUAL_ID: - # Try a manually-entered ID. - prop = manual_id(task) - candidates = prop.candidates - rec = prop.recommendation - # Plugin-provided choices. We invoke the associated callback # function. - elif choice in extra_choices: + elif choice in choices: post_choice = choice.callback(self, task) if isinstance(post_choice, importer.action): - # MANUAL and MANUAL_ID have no effect, even if returned. return post_choice + elif isinstance(post_choice, autotag.Proposal): + # Use the new candidates and continue around the loop. + candidates = post_choice.candidates + rec = post_choice.recommendation # Otherwise, we have a specific match selection. else: @@ -813,11 +791,15 @@ class TerminalImportSession(importer.ImportSession): return action while True: - extra_choices = self._get_plugin_choices(task) + choices = [ + PromptChoice(u'e', u'Enter search', manual_search), + PromptChoice(u'i', u'enter Id', manual_id), + ] + choices += self._get_plugin_choices(task) # Ask for a choice. choice = choose_candidate(candidates, True, rec, item=task.item, - extra_choices=extra_choices) + extra_choices=choices) if choice in (importer.action.SKIP, importer.action.ASIS): return choice @@ -825,23 +807,13 @@ class TerminalImportSession(importer.ImportSession): elif choice == importer.action.TRACKS: assert False # TRACKS is only legal for albums. - elif choice == importer.action.MANUAL: - # Continue in the loop with a new set of candidates. - prop = manual_search(task) - candidates = prop.candidates - rec = prop.recommendation - - elif choice == importer.action.MANUAL_ID: - # Ask for a track ID. - prop = manual_id(task) - candidates = prop.candidates - rec = prop.recommendation - - elif choice in extra_choices: + elif choice in choices: post_choice = choice.callback(self, task) if isinstance(post_choice, importer.action): - # MANUAL and MANUAL_ID have no effect, even if returned. return post_choice + elif isinstance(post_choice, autotag.Proposal): + candidates = post_choice.candidates + rec = post_choice.recommendation else: # Chose a candidate. diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 80409d5f5..ba1b3e250 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -593,7 +593,4 @@ by the choices on the core importer prompt, and hence should not be used: 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 architecture of the import -process. +will be passed to the main loop upon the callback has been processed. From 8937eec50a36a8556fd7789529f6cbd2518ca734 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:15:40 -0500 Subject: [PATCH 20/34] Refactor built-in stnadard choices --- beets/ui/commands.py | 38 ++++++++++++++++++-------------------- docs/dev/plugins.rst | 5 +++-- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 82f5da686..dbfd987d0 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -733,17 +733,11 @@ class TerminalImportSession(importer.ImportSession): # Loop until we have a choice. candidates, rec = task.candidates, task.rec while True: - # Set up menu choices. - choices = [ - PromptChoice(u'e', u'Enter search', manual_search), - PromptChoice(u'i', u'enter Id', manual_id), - ] - choices += self._get_plugin_choices(task) - # Ask for a choice from the user. The result of # `choose_candidate` may be an `importer.action`, an # `AlbumMatch` object for a specific selection, or a # `PromptChoice`. + choices = self._get_choices(task) choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, itemcount=len(task.items), extra_choices=choices @@ -791,13 +785,8 @@ class TerminalImportSession(importer.ImportSession): return action while True: - choices = [ - PromptChoice(u'e', u'Enter search', manual_search), - PromptChoice(u'i', u'enter Id', manual_id), - ] - choices += self._get_plugin_choices(task) - # Ask for a choice. + choices = self._get_choices(task) choice = choose_candidate(candidates, True, rec, item=task.item, extra_choices=choices) @@ -866,8 +855,10 @@ class TerminalImportSession(importer.ImportSession): u"was interrupted. Resume (Y/n)?" .format(displayable_path(path))) - def _get_plugin_choices(self, task): - """Get the extra choices appended to the plugins to the ui prompt. + def _get_choices(self, task): + """Get the list of prompt choices that should be presented to the + user. This consists of both built-in choices and ones provided by + plugins. The `before_choose_candidate` event is sent to the plugins, with session and task as its parameters. Plugins are responsible for @@ -880,18 +871,24 @@ class TerminalImportSession(importer.ImportSession): Returns a list of `PromptChoice`s. """ + # Standard, built-in choices. + choices = [ + PromptChoice(u'e', u'Enter search', manual_search), + PromptChoice(u'i', u'enter Id', manual_id), + ] + # 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. + # Add "dummy" choices for the other baked-in options, for + # duplicate checking. all_choices = [PromptChoice(u'a', u'Apply', None), PromptChoice(u's', u'Skip', None), PromptChoice(u'u', u'Use as-is', None), PromptChoice(u't', u'as Tracks', None), PromptChoice(u'g', u'Group albums', None), - PromptChoice(u'e', u'Enter search', None), - PromptChoice(u'i', u'enter Id', None), - PromptChoice(u'b', u'aBort', None)] +\ + PromptChoice(u'b', u'aBort', None)] + \ + choices + \ extra_choices short_letters = [c.short for c in all_choices] @@ -907,7 +904,8 @@ class TerminalImportSession(importer.ImportSession): u"with '{1}' (short letter: '{2}')", c.long, dup_choices[0].long, c.short) extra_choices.remove(c) - return extra_choices + + return choices + extra_choices # The import command. diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index ba1b3e250..fb063aee0 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -592,5 +592,6 @@ by the choices on the core importer prompt, and hence 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. +be performed by returning a ``importer.action`` value. It may also return a +``autotag.Proposal`` value to update the set of current proposals to be +considered. From 6afe407f45721a31651622b82e722f70b69b78ef Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:16:33 -0500 Subject: [PATCH 21/34] Remove one last duplicated set of prompts --- beets/ui/commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index dbfd987d0..9d1ba7322 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -528,8 +528,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, if not candidates: if singleton: print_(u"No matching recordings found.") - opts = (u'Use as-is', u'Skip', u'Enter search', u'enter Id', - u'aBort') + opts = (u'Use as-is', u'Skip', u'aBort') else: print_(u"No matching release found for {0} tracks." .format(itemcount)) From 3ede874e9653a98fc461d566e956c5bd0bf7af4e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:18:27 -0500 Subject: [PATCH 22/34] Changelog note about new prompt interface --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index f3124271c..116f564df 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ Fixes: * :doc:`/plugins/bpd`: Fix a crash on non-ASCII MPD commands. :bug:`2332` +For plugin developers: new importer prompt choices (see :ref:`append_prompt_choices`), you can now provide new candidates for the user to consider. + 1.4.2 (December 16, 2016) ------------------------- From a357cc4e1a4349dc3cce53a01358c2b650cccbe8 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:20:39 -0500 Subject: [PATCH 23/34] Fix tests for new prompt order --- test/test_plugins.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_plugins.py b/test/test_plugins.py index 7c32e9aca..9aac5f608 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -444,8 +444,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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') + (u'Foo', u'baR') + u'as Tracks', u'Group albums', u'aBort', u'Enter search', + u'enter Id') + (u'Foo', u'baR') self.importer.add_choice(action.SKIP) self.importer.run() @@ -467,8 +467,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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') + (u'Foo', u'baR') + u'aBort', u'Enter search', u'enter Id') + \ + (u'Foo', u'baR') config['import']['singletons'] = True self.importer.add_choice(action.SKIP) @@ -493,8 +493,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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') + (u'baZ',) + u'as Tracks', u'Group albums', u'aBort', u'Enter search', + u'enter Id',) + (u'baZ',) self.importer.add_choice(action.SKIP) self.importer.run() self.mock_input_options.assert_called_once_with(opts, default='a', @@ -517,8 +517,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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') + (u'Foo',) + u'as Tracks', u'Group albums', u'aBort', u'Enter search', + u'enter Id') + (u'Foo',) # DummyPlugin.foo() should be called once with patch.object(DummyPlugin, 'foo', autospec=True) as mock_foo: @@ -548,8 +548,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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') + (u'Foo',) + u'as Tracks', u'Group albums', u'aBort', u'Enter search', + u'enter Id') + (u'Foo',) # DummyPlugin.foo() should be called once with helper.control_stdin('f\n'): From e5a12615e479bf68da95e88971c2f38783063e8f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:32:41 -0500 Subject: [PATCH 24/34] Subsume `abort` action into declarative style --- beets/ui/commands.py | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 9d1ba7322..dcc6776b1 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -528,14 +528,13 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, if not candidates: if singleton: print_(u"No matching recordings found.") - opts = (u'Use as-is', u'Skip', u'aBort') + opts = (u'Use as-is', u'Skip') else: print_(u"No matching release found for {0} tracks." .format(itemcount)) print_(u'For help, see: ' u'http://beets.readthedocs.org/en/latest/faq.html#nomatch') - opts = (u'Use as-is', u'as Tracks', u'Group albums', u'Skip', - u'aBort') + opts = (u'Use as-is', u'as Tracks', u'Group albums', u'Skip') sel = ui.input_options(opts + extra_opts) if sel == u'u': return importer.action.ASIS @@ -544,8 +543,6 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.TRACKS elif sel == u's': return importer.action.SKIP - elif sel == u'b': - raise importer.ImportAbort() elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: @@ -598,10 +595,9 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Ask the user for a choice. if singleton: - opts = (u'Skip', u'Use as-is', u'aBort') + opts = (u'Skip', u'Use as-is') else: - opts = (u'Skip', u'Use as-is', u'as Tracks', u'Group albums', - u'aBort') + opts = (u'Skip', u'Use as-is', u'as Tracks', u'Group albums') sel = ui.input_options(opts + extra_opts, numrange=(1, len(candidates))) if sel == u's': @@ -613,8 +609,6 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u't': assert not singleton return importer.action.TRACKS - elif sel == u'b': - raise importer.ImportAbort() elif sel == u'g': return importer.action.ALBUMS elif sel in extra_actions: @@ -639,11 +633,10 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Ask for confirmation. if singleton: - opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', - u'aBort') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is') else: opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', - u'as Tracks', u'Group albums', u'aBort') + u'as Tracks', u'Group albums') default = config['import']['default_action'].as_choice({ u'apply': u'a', u'skip': u's', @@ -665,8 +658,6 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, elif sel == u't': assert not singleton return importer.action.TRACKS - elif sel == u'b': - raise importer.ImportAbort() elif sel in extra_actions: return extra_actions[sel] @@ -707,6 +698,12 @@ def manual_id(session, task): return autotag.tag_item(task.item, search_ids=search_id.split()) +def abort_action(session, task): + """A prompt choice callback that aborts the importer. + """ + raise importer.ImportAbort() + + class TerminalImportSession(importer.ImportSession): """An import session that runs in a terminal. """ @@ -874,6 +871,7 @@ class TerminalImportSession(importer.ImportSession): choices = [ PromptChoice(u'e', u'Enter search', manual_search), PromptChoice(u'i', u'enter Id', manual_id), + PromptChoice(u'b', u'aBort', abort_action), ] # Send the before_choose_candidate event and flatten list. @@ -881,14 +879,13 @@ class TerminalImportSession(importer.ImportSession): session=self, task=task))) # Add "dummy" choices for the other baked-in options, for # duplicate checking. - all_choices = [PromptChoice(u'a', u'Apply', None), - PromptChoice(u's', u'Skip', None), - PromptChoice(u'u', u'Use as-is', None), - PromptChoice(u't', u'as Tracks', None), - PromptChoice(u'g', u'Group albums', None), - PromptChoice(u'b', u'aBort', None)] + \ - choices + \ - extra_choices + all_choices = [ + PromptChoice(u'a', u'Apply', None), + PromptChoice(u's', u'Skip', None), + PromptChoice(u'u', u'Use as-is', None), + PromptChoice(u't', u'as Tracks', None), + PromptChoice(u'g', u'Group albums', None), + ] + choices + extra_choices short_letters = [c.short for c in all_choices] if len(short_letters) != len(set(short_letters)): From 8763be5423219cd81aa5d742bd04a6d4226a944a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:33:11 -0500 Subject: [PATCH 25/34] Revert "Fix tests for new prompt order" This reverts commit a357cc4e1a4349dc3cce53a01358c2b650cccbe8. We no longer need the tests to change---I was able to fix the order by bringing the "abort" action into the standard list of options. --- test/test_plugins.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_plugins.py b/test/test_plugins.py index 9aac5f608..7c32e9aca 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -444,8 +444,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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'aBort', u'Enter search', - u'enter Id') + (u'Foo', u'baR') + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + (u'Foo', u'baR') self.importer.add_choice(action.SKIP) self.importer.run() @@ -467,8 +467,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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'aBort', u'Enter search', u'enter Id') + \ - (u'Foo', u'baR') + u'Enter search', + u'enter Id', u'aBort') + (u'Foo', u'baR') config['import']['singletons'] = True self.importer.add_choice(action.SKIP) @@ -493,8 +493,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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'aBort', u'Enter search', - u'enter Id',) + (u'baZ',) + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + (u'baZ',) self.importer.add_choice(action.SKIP) self.importer.run() self.mock_input_options.assert_called_once_with(opts, default='a', @@ -517,8 +517,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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'aBort', u'Enter search', - u'enter Id') + (u'Foo',) + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + (u'Foo',) # DummyPlugin.foo() should be called once with patch.object(DummyPlugin, 'foo', autospec=True) as mock_foo: @@ -548,8 +548,8 @@ class PromptChoicesTest(TerminalImportSessionSetup, unittest.TestCase, 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'aBort', u'Enter search', - u'enter Id') + (u'Foo',) + u'as Tracks', u'Group albums', u'Enter search', + u'enter Id', u'aBort') + (u'Foo',) # DummyPlugin.foo() should be called once with helper.control_stdin('f\n'): From 9dff841afe403db91349ba76d5b79998e9cd779c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:33:55 -0500 Subject: [PATCH 26/34] Fix class anme of PromptChoice --- beets/ui/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index dcc6776b1..0911d78d0 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -42,7 +42,7 @@ from beets.util.confit import _package_path import six VARIOUS_ARTISTS = u'Various Artists' -PromptChoice = namedtuple('ExtraChoice', ['short', 'long', 'callback']) +PromptChoice = namedtuple('PromptChoice', ['short', 'long', 'callback']) # Global logger. log = logging.getLogger('beets') From 1e8be0a19fdf525da81cbdcb65f1b014bed0bacc Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:40:06 -0500 Subject: [PATCH 27/34] Use declarative style for ALBUMS/TRACKS choices This unifies the set of choices that are shown at each prompt variant in `choose_candidate`, making its code much shorter. Declarative programming FTW. --- beets/ui/commands.py | 46 ++++++++++++-------------------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 0911d78d0..0c50314d2 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -509,7 +509,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, to the user. Returns one of the following: - * the result of the choice, which may be SKIP, ASIS, or TRACKS + * the result of the choice, which may be SKIP or ASIS * a candidate (an AlbumMatch/TrackMatch object) * a chosen `PromptChoice` from `extra_choices` """ @@ -528,23 +528,17 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, if not candidates: if singleton: print_(u"No matching recordings found.") - opts = (u'Use as-is', u'Skip') else: print_(u"No matching release found for {0} tracks." .format(itemcount)) print_(u'For help, see: ' u'http://beets.readthedocs.org/en/latest/faq.html#nomatch') - opts = (u'Use as-is', u'as Tracks', u'Group albums', u'Skip') + opts = (u'Use as-is', u'Skip') sel = ui.input_options(opts + extra_opts) if sel == u'u': return importer.action.ASIS - elif sel == u't': - assert not singleton - return importer.action.TRACKS elif sel == u's': return importer.action.SKIP - elif sel == u'g': - return importer.action.ALBUMS elif sel in extra_actions: return extra_actions[sel] else: @@ -594,10 +588,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, print_(u' '.join(line)) # Ask the user for a choice. - if singleton: - opts = (u'Skip', u'Use as-is') - else: - opts = (u'Skip', u'Use as-is', u'as Tracks', u'Group albums') + opts = (u'Skip', u'Use as-is') sel = ui.input_options(opts + extra_opts, numrange=(1, len(candidates))) if sel == u's': @@ -606,11 +597,6 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return importer.action.ASIS elif sel == u'm': pass - elif sel == u't': - assert not singleton - return importer.action.TRACKS - elif sel == u'g': - return importer.action.ALBUMS elif sel in extra_actions: return extra_actions[sel] else: # Numerical selection. @@ -632,11 +618,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return match # Ask for confirmation. - if singleton: - opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is') - else: - opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is', - u'as Tracks', u'Group albums') + opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is') default = config['import']['default_action'].as_choice({ u'apply': u'a', u'skip': u's', @@ -649,15 +631,10 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, default=default) if sel == u'a': return match - elif sel == u'g': - return importer.action.ALBUMS elif sel == u's': return importer.action.SKIP elif sel == u'u': return importer.action.ASIS - elif sel == u't': - assert not singleton - return importer.action.TRACKS elif sel in extra_actions: return extra_actions[sel] @@ -740,8 +717,7 @@ class TerminalImportSession(importer.ImportSession): ) # Basic choices that require no more action here. - if choice in (importer.action.SKIP, importer.action.ASIS, - importer.action.TRACKS, importer.action.ALBUMS): + if choice in (importer.action.SKIP, importer.action.ASIS): # Pass selection to main control flow. return choice @@ -789,9 +765,6 @@ class TerminalImportSession(importer.ImportSession): if choice in (importer.action.SKIP, importer.action.ASIS): return choice - elif choice == importer.action.TRACKS: - assert False # TRACKS is only legal for albums. - elif choice in choices: post_choice = choice.callback(self, task) if isinstance(post_choice, importer.action): @@ -873,6 +846,13 @@ class TerminalImportSession(importer.ImportSession): PromptChoice(u'i', u'enter Id', manual_id), PromptChoice(u'b', u'aBort', abort_action), ] + if task.is_album: + choices = [ + PromptChoice(u't', u'as Tracks', + lambda s, t: importer.action.TRACKS), + PromptChoice(u'g', u'Group albums', + lambda s, t: importer.action.ALBUMS), + ] + choices # Send the before_choose_candidate event and flatten list. extra_choices = list(chain(*plugins.send('before_choose_candidate', @@ -883,8 +863,6 @@ class TerminalImportSession(importer.ImportSession): PromptChoice(u'a', u'Apply', None), PromptChoice(u's', u'Skip', None), PromptChoice(u'u', u'Use as-is', None), - PromptChoice(u't', u'as Tracks', None), - PromptChoice(u'g', u'Group albums', None), ] + choices + extra_choices short_letters = [c.short for c in all_choices] From f5e4853bb3b027360c17053e5b791dac97d7b647 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:46:33 -0500 Subject: [PATCH 28/34] Move as-is and skip choices to declarative choices That's all the standard choices that don't depend on which prompt you're looking at! Woohoo! --- beets/ui/commands.py | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 0c50314d2..f8b48568e 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -533,13 +533,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, .format(itemcount)) print_(u'For help, see: ' u'http://beets.readthedocs.org/en/latest/faq.html#nomatch') - opts = (u'Use as-is', u'Skip') - sel = ui.input_options(opts + extra_opts) - if sel == u'u': - return importer.action.ASIS - elif sel == u's': - return importer.action.SKIP - elif sel in extra_actions: + sel = ui.input_options(extra_opts) + if sel in extra_actions: return extra_actions[sel] else: assert False @@ -588,14 +583,9 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, print_(u' '.join(line)) # Ask the user for a choice. - opts = (u'Skip', u'Use as-is') - sel = ui.input_options(opts + extra_opts, + sel = ui.input_options(extra_opts, numrange=(1, len(candidates))) - if sel == u's': - return importer.action.SKIP - elif sel == u'u': - return importer.action.ASIS - elif sel == u'm': + if sel == u'm': pass elif sel in extra_actions: return extra_actions[sel] @@ -618,7 +608,6 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, return match # Ask for confirmation. - opts = (u'Apply', u'More candidates', u'Skip', u'Use as-is') default = config['import']['default_action'].as_choice({ u'apply': u'a', u'skip': u's', @@ -627,14 +616,10 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, }) if default is None: require = True - sel = ui.input_options(opts + extra_opts, require=require, - default=default) + sel = ui.input_options((u'Apply', u'More candidates') + extra_opts, + require=require, default=default) if sel == u'a': return match - elif sel == u's': - return importer.action.SKIP - elif sel == u'u': - return importer.action.ASIS elif sel in extra_actions: return extra_actions[sel] @@ -842,6 +827,10 @@ class TerminalImportSession(importer.ImportSession): """ # Standard, built-in choices. choices = [ + PromptChoice(u's', u'Skip', + lambda s, t: importer.action.SKIP), + PromptChoice(u'u', u'Use as-is', + lambda s, t: importer.action.ASIS), PromptChoice(u'e', u'Enter search', manual_search), PromptChoice(u'i', u'enter Id', manual_id), PromptChoice(u'b', u'aBort', abort_action), @@ -857,14 +846,14 @@ class TerminalImportSession(importer.ImportSession): # Send the before_choose_candidate event and flatten list. extra_choices = list(chain(*plugins.send('before_choose_candidate', session=self, task=task))) - # Add "dummy" choices for the other baked-in options, for + + # Add a "dummy" choice for the other baked-in option, for # duplicate checking. all_choices = [ PromptChoice(u'a', u'Apply', None), - PromptChoice(u's', u'Skip', None), - PromptChoice(u'u', u'Use as-is', None), ] + choices + extra_choices + # Check for conflicts. short_letters = [c.short for c in all_choices] if len(short_letters) != len(set(short_letters)): # Duplicate short letter has been found. From c98972894a24575477d4438091a3529c9a88dcfa Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:49:26 -0500 Subject: [PATCH 29/34] Better prompt order --- beets/ui/commands.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index f8b48568e..723c3f27b 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -830,18 +830,20 @@ class TerminalImportSession(importer.ImportSession): PromptChoice(u's', u'Skip', lambda s, t: importer.action.SKIP), PromptChoice(u'u', u'Use as-is', - lambda s, t: importer.action.ASIS), - PromptChoice(u'e', u'Enter search', manual_search), - PromptChoice(u'i', u'enter Id', manual_id), - PromptChoice(u'b', u'aBort', abort_action), + lambda s, t: importer.action.ASIS) ] if task.is_album: - choices = [ + choices += [ PromptChoice(u't', u'as Tracks', lambda s, t: importer.action.TRACKS), PromptChoice(u'g', u'Group albums', lambda s, t: importer.action.ALBUMS), - ] + choices + ] + choices += [ + PromptChoice(u'e', u'Enter search', manual_search), + PromptChoice(u'i', u'enter Id', manual_id), + PromptChoice(u'b', u'aBort', abort_action), + ] # Send the before_choose_candidate event and flatten list. extra_choices = list(chain(*plugins.send('before_choose_candidate', From 63f50287d589aec5fef0000a357aad9548f35f9c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 14:54:25 -0500 Subject: [PATCH 30/34] Rename some choice-related variables These are no longer "extra"---they're *all* the choices that will be used. --- beets/ui/commands.py | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 723c3f27b..168f0d515 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -495,7 +495,7 @@ def _summary_judgment(rec): def choose_candidate(candidates, singleton, rec, cur_artist=None, cur_album=None, item=None, itemcount=None, - extra_choices=[]): + 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 @@ -503,15 +503,12 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, `cur_album`, and `itemcount` must be provided. For singletons, `item` must be provided. - `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. + `choices` is a list of `PromptChoice`s to be used in each prompt. Returns one of the following: * the result of the choice, which may be SKIP or ASIS * a candidate (an AlbumMatch/TrackMatch object) - * a chosen `PromptChoice` from `extra_choices` + * a chosen `PromptChoice` from `choices` """ # Sanity check. if singleton: @@ -520,9 +517,9 @@ 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. - extra_opts = tuple(c.long for c in extra_choices) - extra_actions = {c.short: c for c in extra_choices} + # Build helper variables for the prompt choices. + choice_opts = tuple(c.long for c in choices) + choice_actions = {c.short: c for c in choices} # Zero candidates. if not candidates: @@ -533,9 +530,9 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, .format(itemcount)) print_(u'For help, see: ' u'http://beets.readthedocs.org/en/latest/faq.html#nomatch') - sel = ui.input_options(extra_opts) - if sel in extra_actions: - return extra_actions[sel] + sel = ui.input_options(choice_opts) + if sel in choice_actions: + return choice_actions[sel] else: assert False @@ -583,12 +580,12 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, print_(u' '.join(line)) # Ask the user for a choice. - sel = ui.input_options(extra_opts, + sel = ui.input_options(choice_opts, numrange=(1, len(candidates))) if sel == u'm': pass - elif sel in extra_actions: - return extra_actions[sel] + elif sel in choice_actions: + return choice_actions[sel] else: # Numerical selection. match = candidates[sel - 1] if sel != 1: @@ -616,12 +613,12 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, }) if default is None: require = True - sel = ui.input_options((u'Apply', u'More candidates') + extra_opts, + sel = ui.input_options((u'Apply', u'More candidates') + choice_opts, require=require, default=default) if sel == u'a': return match - elif sel in extra_actions: - return extra_actions[sel] + elif sel in choice_actions: + return choice_actions[sel] def manual_search(session, task): @@ -698,7 +695,7 @@ class TerminalImportSession(importer.ImportSession): choices = self._get_choices(task) choice = choose_candidate( candidates, False, rec, task.cur_artist, task.cur_album, - itemcount=len(task.items), extra_choices=choices + itemcount=len(task.items), choices=choices ) # Basic choices that require no more action here. @@ -745,7 +742,7 @@ class TerminalImportSession(importer.ImportSession): # Ask for a choice. choices = self._get_choices(task) choice = choose_candidate(candidates, True, rec, item=task.item, - extra_choices=choices) + choices=choices) if choice in (importer.action.SKIP, importer.action.ASIS): return choice From c7351d9086914b6a314ced7fac1be136dfc17b20 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 16:18:02 -0500 Subject: [PATCH 31/34] Flip an erroneous execute bit --- beets/library.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 beets/library.py diff --git a/beets/library.py b/beets/library.py old mode 100755 new mode 100644 From 24d43635cd6862ae82a780fc01af8ae85ef1e3bb Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 16:33:43 -0500 Subject: [PATCH 32/34] Slightly more helpful UnreadableFileError (#2351) The exception now says something about what caused it, instead of just listing the path that led to the error. --- beets/mediafile.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index ad66b74c5..0f595a36a 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -87,8 +87,8 @@ PREFERRED_IMAGE_EXTENSIONS = {'jpeg': 'jpg'} class UnreadableFileError(Exception): """Mutagen is not able to extract information from the file. """ - def __init__(self, path): - Exception.__init__(self, repr(path)) + def __init__(self, path, msg): + Exception.__init__(self, msg if msg else repr(path)) class FileTypeError(UnreadableFileError): @@ -132,7 +132,7 @@ def mutagen_call(action, path, func, *args, **kwargs): return func(*args, **kwargs) except mutagen.MutagenError as exc: log.debug(u'%s failed: %s', action, six.text_type(exc)) - raise UnreadableFileError(path) + raise UnreadableFileError(path, six.text_type(exc)) except Exception as exc: # Isolate bugs in Mutagen. log.debug(u'%s', traceback.format_exc()) From 413666a82531bed46291f6e458aa78b92b1ee9c8 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Dec 2016 16:36:51 -0500 Subject: [PATCH 33/34] Fix #2351: crash in scrub on MediaFile errors --- beetsplug/scrub.py | 1 + docs/changelog.rst | 2 ++ 2 files changed, 3 insertions(+) diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index 4dcefe572..8ecf8ad5d 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -122,6 +122,7 @@ class ScrubPlugin(BeetsPlugin): except mediafile.UnreadableFileError as exc: self._log.error(u'could not open file to scrub: {0}', exc) + return art = mf.art # Remove all tags. diff --git a/docs/changelog.rst b/docs/changelog.rst index 116f564df..e5e027016 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,8 @@ Features: Fixes: * :doc:`/plugins/bpd`: Fix a crash on non-ASCII MPD commands. :bug:`2332` +* :doc:`/plugins/scrub`: Avoid a crash when files cannot be read or written. + :bug:`2351` For plugin developers: new importer prompt choices (see :ref:`append_prompt_choices`), you can now provide new candidates for the user to consider. From 8bb703619f5ecdeb98c637cd46fea2b6e25bf6e5 Mon Sep 17 00:00:00 2001 From: dopefishh Date: Thu, 29 Dec 2016 11:38:27 +0100 Subject: [PATCH 34/34] use util.py3_path for web attachment filenames (#2353) Web attachment filenames must be passed as a string for Python 3 --- beetsplug/web/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/web/__init__.py b/beetsplug/web/__init__.py index 810de8718..e7b9ec81f 100644 --- a/beetsplug/web/__init__.py +++ b/beetsplug/web/__init__.py @@ -206,7 +206,7 @@ def item_file(item_id): response = flask.send_file( util.py3_path(item.path), as_attachment=True, - attachment_filename=os.path.basename(item.path), + attachment_filename=os.path.basename(util.py3_path(item.path)), ) response.headers['Content-Length'] = os.path.getsize(item.path) return response