diff --git a/beets/importer.py b/beets/importer.py index 868ac6922..f4dd4853d 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -45,7 +45,10 @@ from beets import mediafile action = Enum('action', ['SKIP', 'ASIS', 'TRACKS', 'MANUAL', 'APPLY', 'MANUAL_ID', - 'ALBUMS']) + '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. QUEUE_SIZE = 128 SINGLE_ARTIST_THRESH = 0.25 @@ -443,7 +446,8 @@ class ImportTask(BaseImportTask): # 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): + if choice in (action.SKIP, action.ASIS, action.TRACKS, action.ALBUMS, + action.RETAG): self.choice_flag = choice self.match = None else: @@ -479,10 +483,10 @@ class ImportTask(BaseImportTask): """Returns identifying metadata about the current choice. For albums, this is an (artist, album) pair. For items, this is (artist, title). May only be called when the choice flag is ASIS - (in which case the data comes from the files' current metadata) - or APPLY (data comes from the choice). + or RETAG (in which case the data comes from the files' current + metadata) or APPLY (data comes from the choice). """ - if self.choice_flag is action.ASIS: + if self.choice_flag in (action.ASIS, action.RETAG): return (self.cur_artist, self.cur_album) elif self.choice_flag is action.APPLY: return (self.match.info.artist, self.match.info.album) @@ -493,7 +497,7 @@ class ImportTask(BaseImportTask): If the tasks applies an album match the method only returns the matched items. """ - if self.choice_flag == action.ASIS: + if self.choice_flag in (action.ASIS, action.RETAG): return list(self.items) elif self.choice_flag == action.APPLY: return self.match.mapping.keys() @@ -617,7 +621,10 @@ class ImportTask(BaseImportTask): return duplicates def align_album_level_fields(self): - """Make some album fields equal across `self.items`. + """Make some album fields equal across `self.items`. For the + RETAG action, we assume that the responsible for returning it + (ie. a plugin) always ensures that the first item contains + valid data on the relevant fields. """ changes = {} @@ -637,7 +644,7 @@ class ImportTask(BaseImportTask): changes['albumartist'] = config['va_name'].get(unicode) changes['comp'] = True - elif self.choice_flag == action.APPLY: + elif self.choice_flag in (action.APPLY, action.RETAG): # Applying autotagged metadata. Just get AA from the first # item. if not self.items[0].albumartist: @@ -672,7 +679,7 @@ class ImportTask(BaseImportTask): # old paths. item.move(copy, link) - if write and self.apply: + if write and (self.apply or self.choice_flag == action.RETAG): item.try_write() with session.lib.transaction(): @@ -807,8 +814,8 @@ class SingletonImportTask(ImportTask): self.paths = [item.path] def chosen_ident(self): - assert self.choice_flag in (action.ASIS, action.APPLY) - if self.choice_flag is action.ASIS: + assert self.choice_flag in (action.ASIS, action.APPLY, action.RETAG) + if self.choice_flag in (action.ASIS, action.RETAG): return (self.item.artist, self.item.title) elif self.choice_flag is action.APPLY: return (self.match.info.artist, self.match.info.title) @@ -1315,7 +1322,7 @@ def resolve_duplicates(session, task): """Check if a task conflicts with items or albums already imported and ask the session to resolve this. """ - if task.choice_flag in (action.ASIS, action.APPLY): + if task.choice_flag in (action.ASIS, action.APPLY, action.RETAG): found_duplicates = task.find_duplicates(session.lib) if found_duplicates: log.debug('found duplicates: {}'.format( diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 192c920ef..1bd58fbd5 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -870,7 +870,9 @@ class TerminalImportSession(importer.ImportSession): 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. + If two or more choices have the same short letter, a warning is + emitted and all but one choices are discarded, giving preference + to the default importer choices. Returns a list of `PromptChoice`s. """ diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 29676fa08..ee045664e 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -21,7 +21,9 @@ from beets import plugins from beets import util from beets import ui from beets.dbcore import types -from beets.ui.commands import _do_query +from beets.importer import action +from beets.ui.commands import _do_query, PromptChoice +from copy import deepcopy import subprocess import yaml from tempfile import NamedTemporaryFile @@ -119,7 +121,7 @@ def flatten(obj, fields): return d -def apply(obj, data): +def apply_(obj, data): """Set the fields of a `dbcore.Model` object according to a dictionary. @@ -151,6 +153,9 @@ class EditPlugin(plugins.BeetsPlugin): 'ignore_fields': 'id path', }) + self.register_listener('before_choose_candidate', + self.before_choose_candidate_listener) + def commands(self): edit_command = ui.Subcommand( 'edit', @@ -262,10 +267,14 @@ class EditPlugin(plugins.BeetsPlugin): return False # Show the changes. + # If the objects are not on the DB yet, we need a copy of their + # original state for show_model_changes. + objs_old = [deepcopy(obj) if not obj._db else None + for obj in objs] self.apply_data(objs, old_data, new_data) changed = False - for obj in objs: - changed |= ui.show_model_changes(obj) + for obj, obj_old in zip(objs, objs_old): + changed |= ui.show_model_changes(obj, obj_old) if not changed: ui.print_('No changes to apply.') return False @@ -313,8 +322,8 @@ class EditPlugin(plugins.BeetsPlugin): if forbidden: continue - id = int(old_dict['id']) - apply(obj_by_id[id], new_dict) + id_ = int(old_dict['id']) + apply_(obj_by_id[id_], new_dict) def save_changes(self, objs): """Save a list of updated Model objects to the database. @@ -324,3 +333,55 @@ class EditPlugin(plugins.BeetsPlugin): if ob._dirty: self._log.debug('saving changes to {}', ob) ob.try_sync(ui.should_write(), ui.should_move()) + + # Methods for interactive importer execution. + + def before_choose_candidate_listener(self, session, task): + """Append an "Edit" choice and an "edit Candidates" choice (if + there are candidates) to the interactive importer prompt. + """ + choices = [PromptChoice('d', 'eDit', self.importer_edit)] + if task.candidates: + choices.append(PromptChoice('c', 'edit Candidates', + self.importer_edit_candidate)) + + return choices + + def importer_edit(self, session, task): + """Callback for invoking the functionality during an interactive + import session on the *original* item tags. + """ + # Assign temporary ids to the Items. + for i, obj in enumerate(task.items): + obj.id = i + 1 + + # Present the YAML to the user and let her change it. + fields = self._get_fields(album=False, extra=[]) + success = self.edit_objects(task.items, fields) + + # Remove temporary ids. + for obj in task.items: + obj.id = None + + # Save the new data. + if success: + # Return action.RETAG, which makes the importer write the tags + # to the files if needed without re-applying metadata. + return action.RETAG + else: + # Edit cancelled / no edits made. Revert changes. + for obj in task.items: + obj.read() + + def importer_edit_candidate(self, session, task): + """Callback for invoking the functionality during an interactive + import session on a *candidate*. The candidate's metadata is + applied to the original items. + """ + # Prompt the user for a candidate. + sel = ui.input_options([], numrange=(1, len(task.candidates))) + # Force applying the candidate on the items. + task.match = task.candidates[sel - 1] + task.apply_metadata() + + return self.importer_edit(session, task) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index b7c5d82f9..80409d5f5 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -585,10 +585,11 @@ 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: ``a``, ``s``, ``u``, ``t``, ``g``, ``e``, ``i``, ``b``. +plugin is not already in use: the importer will emit a warning and discard +all but one of the choices using the same letter, giving priority to the +core importer prompt choices. As a reference, the following characters are used +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 diff --git a/docs/plugins/edit.rst b/docs/plugins/edit.rst index 507d56950..44286f79c 100644 --- a/docs/plugins/edit.rst +++ b/docs/plugins/edit.rst @@ -23,6 +23,30 @@ The ``edit`` command has these command-line options: (in addition to the defaults set in the configuration). - ``--all``: Edit *all* available fields. +Interactive Usage +----------------- + +The ``edit`` plugin can also be invoked during an import session. If enabled, it +adds two new options to the user prompt:: + + [A]pply, More candidates, Skip, Use as-is, as Tracks, Group albums, Enter search, enter Id, aBort, eDit, edit Candidates? + +- ``eDit``: use this option for using the original items' metadata as the + starting point for your edits. +- ``edit Candidates``: use this option for using a candidate's metadata as the + starting point for your edits. + +Please note that currently the interactive usage of the plugin will only allow +you to change the item-level fields. In case you need to edit the album-level +fields, the recommended approach is to invoke the plugin via the command line +in album mode (``beet edit -a QUERY``) after the import. + +Also, please be aware that the ``edit Candidates`` choice can only be used with +the matches found during the initial search (and currently not supporting the +candidates found via the ``Enter search`` or ``enter Id`` choices). You might +find the ``--search-id SEARCH_ID`` :ref:`import-cmd` option useful for those +cases where you already have a specific candidate ID that you want to edit. + Configuration ------------- diff --git a/test/test_edit.py b/test/test_edit.py index 25b24cea0..c35c71c08 100644 --- a/test/test_edit.py +++ b/test/test_edit.py @@ -20,11 +20,16 @@ from mock import patch from test import _common from test._common import unittest from test.helper import TestHelper, control_stdin +from test.test_ui_importer import TerminalImportSessionSetup +from test.test_importer import ImportHelper, AutotagStub +from beets.library import Item +from beetsplug.edit import EditPlugin class ModifyFileMocker(object): """Helper for modifying a file, replacing or editing its contents. Used for - mocking the calls to the external editor during testing.""" + mocking the calls to the external editor during testing. + """ def __init__(self, contents=None, replacements=None): """ `self.contents` and `self.replacements` are initalized here, in @@ -63,52 +68,8 @@ class ModifyFileMocker(object): f.write(contents) -@_common.slow_test() -class EditCommandTest(unittest.TestCase, TestHelper): - """ Black box tests for `beetsplug.edit`. Command line interaction is - simulated using `test.helper.control_stdin()`, and yaml editing via an - external editor is simulated using `ModifyFileMocker`. - """ - ALBUM_COUNT = 1 - TRACK_COUNT = 10 - - def setUp(self): - self.setup_beets() - self.load_plugins('edit') - # make sure that we avoid invoking the editor except for making changes - self.config['edit']['diff_method'] = '' - # add an album, storing the original fields for comparison - self.album = self.add_album_fixture(track_count=self.TRACK_COUNT) - self.album_orig = {f: self.album[f] for f in self.album._fields} - self.items_orig = [{f: item[f] for f in item._fields} for - item in self.album.items()] - - # keep track of write()s - self.write_patcher = patch('beets.library.Item.write') - self.mock_write = self.write_patcher.start() - - def tearDown(self): - self.write_patcher.stop() - self.teardown_beets() - self.unload_plugins() - - def run_mocked_command(self, modify_file_args={}, stdin=[], args=[]): - """Run the edit command, with mocked stdin and yaml writing, and - passing `args` to `run_command`.""" - m = ModifyFileMocker(**modify_file_args) - with patch('beetsplug.edit.edit', side_effect=m.action): - with control_stdin('\n'.join(stdin)): - self.run_command('edit', *args) - - def assertCounts(self, album_count=ALBUM_COUNT, track_count=TRACK_COUNT, - write_call_count=TRACK_COUNT, title_starts_with=''): - """Several common assertions on Album, Track and call counts.""" - self.assertEqual(len(self.lib.albums()), album_count) - self.assertEqual(len(self.lib.items()), track_count) - self.assertEqual(self.mock_write.call_count, write_call_count) - self.assertTrue(all(i.title.startswith(title_starts_with) - for i in self.lib.items())) - +class EditMixin(object): + """Helper containing some common functionality used for the Edit tests.""" def assertItemFieldsModified(self, library_items, items, fields=[], allowed=['path']): """Assert that items in the library (`lib_items`) have different values @@ -125,9 +86,64 @@ class EditCommandTest(unittest.TestCase, TestHelper): self.assertEqual(set(diff_fields).difference(allowed), set(fields)) + def run_mocked_interpreter(self, modify_file_args={}, stdin=[]): + """Run the edit command during an import session, with mocked stdin and + yaml writing. + """ + m = ModifyFileMocker(**modify_file_args) + with patch('beetsplug.edit.edit', side_effect=m.action): + with control_stdin('\n'.join(stdin)): + self.importer.run() + + def run_mocked_command(self, modify_file_args={}, stdin=[], args=[]): + """Run the edit command, with mocked stdin and yaml writing, and + passing `args` to `run_command`.""" + m = ModifyFileMocker(**modify_file_args) + with patch('beetsplug.edit.edit', side_effect=m.action): + with control_stdin('\n'.join(stdin)): + self.run_command('edit', *args) + + +@_common.slow_test() +class EditCommandTest(unittest.TestCase, TestHelper, EditMixin): + """Black box tests for `beetsplug.edit`. Command line interaction is + simulated using `test.helper.control_stdin()`, and yaml editing via an + external editor is simulated using `ModifyFileMocker`. + """ + ALBUM_COUNT = 1 + TRACK_COUNT = 10 + + def setUp(self): + self.setup_beets() + self.load_plugins('edit') + # Add an album, storing the original fields for comparison. + self.album = self.add_album_fixture(track_count=self.TRACK_COUNT) + self.album_orig = {f: self.album[f] for f in self.album._fields} + self.items_orig = [{f: item[f] for f in item._fields} for + item in self.album.items()] + + # Keep track of write()s. + self.write_patcher = patch('beets.library.Item.write') + self.mock_write = self.write_patcher.start() + + def tearDown(self): + EditPlugin.listeners = None + self.write_patcher.stop() + self.teardown_beets() + self.unload_plugins() + + def assertCounts(self, album_count=ALBUM_COUNT, track_count=TRACK_COUNT, + write_call_count=TRACK_COUNT, title_starts_with=''): + """Several common assertions on Album, Track and call counts.""" + self.assertEqual(len(self.lib.albums()), album_count) + self.assertEqual(len(self.lib.items()), track_count) + self.assertEqual(self.mock_write.call_count, write_call_count) + self.assertTrue(all(i.title.startswith(title_starts_with) + for i in self.lib.items())) + def test_title_edit_discard(self): - """Edit title for all items in the library, then discard changes-""" - # edit titles + """Edit title for all items in the library, then discard changes.""" + # Edit track titles. self.run_mocked_command({'replacements': {u't\u00eftle': u'modified t\u00eftle'}}, # Cancel. @@ -139,7 +155,7 @@ class EditCommandTest(unittest.TestCase, TestHelper): def test_title_edit_apply(self): """Edit title for all items in the library, then apply changes.""" - # edit titles + # Edit track titles. self.run_mocked_command({'replacements': {u't\u00eftle': u'modified t\u00eftle'}}, # Apply changes. @@ -152,14 +168,14 @@ class EditCommandTest(unittest.TestCase, TestHelper): def test_single_title_edit_apply(self): """Edit title for one item in the library, then apply changes.""" - # edit title + # Edit one track title. self.run_mocked_command({'replacements': {u't\u00eftle 9': u'modified t\u00eftle 9'}}, # Apply changes. ['a']) self.assertCounts(write_call_count=1,) - # no changes except on last item + # No changes except on last item. self.assertItemFieldsModified(list(self.album.items())[:-1], self.items_orig[:-1], []) self.assertEqual(list(self.album.items())[-1].title, @@ -167,9 +183,9 @@ class EditCommandTest(unittest.TestCase, TestHelper): def test_noedit(self): """Do not edit anything.""" - # do not edit anything + # Do not edit anything. self.run_mocked_command({'contents': None}, - # no stdin + # No stdin. []) self.assertCounts(write_call_count=0, @@ -180,7 +196,7 @@ class EditCommandTest(unittest.TestCase, TestHelper): """Edit the album field for all items in the library, apply changes. By design, the album should not be updated."" """ - # edit album + # Edit album. self.run_mocked_command({'replacements': {u'\u00e4lbum': u'modified \u00e4lbum'}}, # Apply changes. @@ -189,14 +205,14 @@ class EditCommandTest(unittest.TestCase, TestHelper): self.assertCounts(write_call_count=self.TRACK_COUNT) self.assertItemFieldsModified(self.album.items(), self.items_orig, ['album']) - # ensure album is *not* modified + # Ensure album is *not* modified. self.album.load() self.assertEqual(self.album.album, u'\u00e4lbum') def test_single_edit_add_field(self): """Edit the yaml file appending an extra field to the first item, then apply changes.""" - # append "foo: bar" to item with id == 1 + # Append "foo: bar" to item with id == 1. self.run_mocked_command({'replacements': {u"id: 1": u"id: 1\nfoo: bar"}}, # Apply changes. @@ -237,7 +253,7 @@ class EditCommandTest(unittest.TestCase, TestHelper): def test_malformed_yaml(self): """Edit the yaml file incorrectly (resulting in a malformed yaml document).""" - # edit the yaml file to an invalid file + # Edit the yaml file to an invalid file. self.run_mocked_command({'contents': '!MALFORMED'}, # Edit again to fix? No. ['n']) @@ -248,15 +264,162 @@ class EditCommandTest(unittest.TestCase, TestHelper): def test_invalid_yaml(self): """Edit the yaml file incorrectly (resulting in a well-formed but invalid yaml document).""" - # edit the yaml file to an invalid file + # Edit the yaml file to an invalid but parseable file. self.run_mocked_command({'contents': 'wellformed: yes, but invalid'}, - # no stdin + # No stdin. []) self.assertCounts(write_call_count=0, title_starts_with=u't\u00eftle') +@_common.slow_test() +class EditDuringImporterTest(TerminalImportSessionSetup, unittest.TestCase, + ImportHelper, TestHelper, EditMixin): + """TODO + """ + IGNORED = ['added', 'album_id', 'id', 'mtime', 'path'] + + def setUp(self): + self.setup_beets() + self.load_plugins('edit') + # Create some mediafiles, and store them for comparison. + self._create_import_dir(3) + self.items_orig = [Item.from_path(f.path) for f in self.media_files] + self.matcher = AutotagStub().install() + self.matcher.matching = AutotagStub.GOOD + self.config['import']['timid'] = True + + def tearDown(self): + EditPlugin.listeners = None + self.unload_plugins() + self.teardown_beets() + self.matcher.restore() + + def test_edit_apply_asis(self): + """Edit the album field for all items in the library, apply changes, + using the original item tags. + """ + self._setup_import_session() + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Tag Title': + u'Edited Title'}}, + # eDit, Apply changes. + ['d', 'a']) + + # Check that only the 'title' field is modified. + self.assertItemFieldsModified(self.lib.items(), self.items_orig, + ['title'], + self.IGNORED + ['albumartist', + 'mb_albumartistid']) + self.assertTrue(all('Edited Title' in i.title + for i in self.lib.items())) + + # Ensure album is *not* fetched from a candidate. + self.assertEqual(self.lib.albums()[0].mb_albumid, u'') + + def test_edit_discard_asis(self): + """Edit the album field for all items in the library, discard changes, + using the original item tags. + """ + self._setup_import_session() + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Tag Title': + u'Edited Title'}}, + # eDit, Cancel, Use as-is. + ['d', 'c', 'u']) + + # Check that nothing is modified, the album is imported ASIS. + self.assertItemFieldsModified(self.lib.items(), self.items_orig, + [], + self.IGNORED + ['albumartist', + 'mb_albumartistid']) + self.assertTrue(all('Tag Title' in i.title + for i in self.lib.items())) + + # Ensure album is *not* fetched from a candidate. + self.assertEqual(self.lib.albums()[0].mb_albumid, u'') + + def test_edit_apply_candidate(self): + """Edit the album field for all items in the library, apply changes, + using a candidate. + """ + self._setup_import_session() + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Applied Title': + u'Edited Title'}}, + # edit Candidates, 1, Apply changes. + ['c', '1', 'a']) + + # Check that 'title' field is modified, and other fields come from + # the candidate. + self.assertTrue(all('Edited Title ' in i.title + for i in self.lib.items())) + self.assertTrue(all('match ' in i.mb_trackid + for i in self.lib.items())) + + # Ensure album is fetched from a candidate. + self.assertIn('albumid', self.lib.albums()[0].mb_albumid) + + def test_edit_discard_candidate(self): + """Edit the album field for all items in the library, discard changes, + using a candidate. + """ + self._setup_import_session() + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Applied Title': + u'Edited Title'}}, + # edit Candidates, 1, Apply changes. + ['c', '1', 'a']) + + # Check that 'title' field is modified, and other fields come from + # the candidate. + self.assertTrue(all('Edited Title ' in i.title + for i in self.lib.items())) + self.assertTrue(all('match ' in i.mb_trackid + for i in self.lib.items())) + + # Ensure album is fetched from a candidate. + self.assertIn('albumid', self.lib.albums()[0].mb_albumid) + + def test_edit_apply_asis_singleton(self): + """Edit the album field for all items in the library, apply changes, + using the original item tags and singleton mode. + """ + self._setup_import_session(singletons=True) + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Tag Title': + u'Edited Title'}}, + # eDit, Apply changes, aBort. + ['d', 'a', 'b']) + + # Check that only the 'title' field is modified. + self.assertItemFieldsModified(self.lib.items(), self.items_orig, + ['title'], + self.IGNORED + ['albumartist', + 'mb_albumartistid']) + self.assertTrue(all('Edited Title' in i.title + for i in self.lib.items())) + + def test_edit_apply_candidate_singleton(self): + """Edit the album field for all items in the library, apply changes, + using a candidate and singleton mode. + """ + self._setup_import_session() + # Edit track titles. + self.run_mocked_interpreter({'replacements': {u'Applied Title': + u'Edited Title'}}, + # edit Candidates, 1, Apply changes, aBort. + ['c', '1', 'a', 'b']) + + # Check that 'title' field is modified, and other fields come from + # the candidate. + self.assertTrue(all('Edited Title ' in i.title + for i in self.lib.items())) + self.assertTrue(all('match ' in i.mb_trackid + for i in self.lib.items())) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)