From b7a2a42d9c9c166dd2629c1c93e8d346d41f3763 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Thu, 28 Jan 2016 13:01:44 +0100 Subject: [PATCH 01/12] Fix docs on plugin prompt choices conflict solving * Fix docstring and `plugins.rst` regarding how the PromptChoices that use the same short letter are handled (they are discarded instead of raising an Exception). --- beets/ui/commands.py | 4 +++- docs/dev/plugins.rst | 9 +++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 3e3acb7c3..41f118bc6 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -853,7 +853,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/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 From 98abe6952048cbb23448fedae1e8ed402020acc8 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Thu, 28 Jan 2016 14:07:10 +0100 Subject: [PATCH 02/12] edit: invoke editor during importer, on Items * Initial draft for invoking the edit plugin during an importer session. * Add prompt choices for editing the original file tags ("eDit") and apply a candidate and then edit ("edit Candidates"). * Modify plugin (_get_fields, apply_data, edit_objects) so "path" can be used as a reference field instead of "id", as the Items are not still on the database when the plugin is invoked via the importer. * Modify ImportTask.manipulate_files() with a temporary flag for writing the item tags even if ASIS was selected. --- beets/importer.py | 3 +- beetsplug/edit.py | 93 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 868ac6922..b8bbed65f 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -672,7 +672,8 @@ class ImportTask(BaseImportTask): # old paths. item.move(copy, link) - if write and self.apply: + # TODO: the EDIT_FLAG field is a hack! + if write and (self.apply or getattr(self, 'EDIT_FLAG', False)): item.try_write() with session.lib.transaction(): diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 29676fa08..a3905e5dd 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 @@ -151,6 +153,13 @@ class EditPlugin(plugins.BeetsPlugin): 'ignore_fields': 'id path', }) + self.register_listener('before_choose_candidate', + self.before_choose_candidate_event) + + # Field to be used as "unequivocal, non-editable key" for an Item. + # TODO: cleanup + self.mapping_field = 'id' + def commands(self): edit_command = ui.Subcommand( 'edit', @@ -202,8 +211,8 @@ class EditPlugin(plugins.BeetsPlugin): if extra: fields += extra - # Ensure we always have the `id` field for identification. - fields.append('id') + # Ensure we always have the mapping field for identification. + fields.append(self.mapping_field) return set(fields) @@ -262,10 +271,21 @@ 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. + if all(not obj.id for obj in objs): + objs_old = deepcopy(objs) self.apply_data(objs, old_data, new_data) changed = False for obj in objs: - changed |= ui.show_model_changes(obj) + if not obj.id: + # TODO: remove uglyness + obj_old = next(x for x in objs_old if + getattr(x, self.mapping_field) == + getattr(obj, self.mapping_field)) + else: + obj_old = None + changed |= ui.show_model_changes(obj, obj_old) if not changed: ui.print_('No changes to apply.') return False @@ -295,11 +315,24 @@ class EditPlugin(plugins.BeetsPlugin): The objects are not written back to the database, so the changes are temporary. """ + # TODO: make this more pythonic + def ref_field_value(o): + if self.mapping_field == 'id': + return int(o.id) + elif self.mapping_field == 'path': + return util.displayable_path(o.path) + + def obj_from_ref(d): + if self.mapping_field == 'id': + return int(d['id']) + elif self.mapping_field == 'path': + return util.displayable_path(d['path']) + if len(old_data) != len(new_data): self._log.warn('number of objects changed from {} to {}', len(old_data), len(new_data)) - obj_by_id = {o.id: o for o in objs} + obj_by_f = {ref_field_value(o): o for o in objs} ignore_fields = self.config['ignore_fields'].as_str_seq() for old_dict, new_dict in zip(old_data, new_data): # Prohibit any changes to forbidden fields to avoid @@ -313,8 +346,9 @@ class EditPlugin(plugins.BeetsPlugin): if forbidden: continue - id = int(old_dict['id']) - apply(obj_by_id[id], new_dict) + # Reconcile back the user edits, using the mapping_field. + val = obj_from_ref(old_dict) + apply(obj_by_f[val], new_dict) def save_changes(self, objs): """Save a list of updated Model objects to the database. @@ -324,3 +358,48 @@ 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_event(self, session, task): + """Append an "Edit" choice to the interactive importer prompt. + """ + return [PromptChoice('d', 'eDit', self.importer_edit), + PromptChoice('c', 'edit Candidates', + self.importer_edit_candidate)] + + def importer_edit(self, session, task): + """Callback for invoking the functionality during an interactive + import session on the *original* item tags. + """ + # Make 'path' the mapping field, as the Items do not have ids yet. + # TODO: move to ~import_begin + self.mapping_field = 'path' + + # Present the YAML to the user and let her change it. + fields = self._get_fields(album=None, extra=[]) + success = self.edit_objects(task.items, fields) + + # Save the new data. + if success: + # TODO: implement properly, this is a quick illustrative hack. + # If using Items, the operation is something like + # "use the *modified* Items AS-IS *and* write() them" + task.EDIT_FLAG = True + return action.ASIS + 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* applied to the original items. + """ + # Prompt the user for a candidate, and simulate matching. + 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) From 30927a901ffeaa279512e2565683314925e62c90 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 29 Jan 2016 19:54:23 +0100 Subject: [PATCH 03/12] Fix flake8 error --- beetsplug/edit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index a3905e5dd..77984b77b 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -399,7 +399,7 @@ class EditPlugin(plugins.BeetsPlugin): # Prompt the user for a candidate, and simulate matching. sel = ui.input_options([], numrange=(1, len(task.candidates))) # Force applying the candidate on the items. - task.match = task.candidates[sel-1] + task.match = task.candidates[sel - 1] task.apply_metadata() return self.importer_edit(session, task) From e23718dc0c826b81abef677d44a73c117914649e Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Mon, 1 Feb 2016 18:51:24 +0100 Subject: [PATCH 04/12] Add importer RETAG action * Add importer "RETAG" action, to represent the case where the files metadata has been modified by some mean other than applying metadata (via a plugin), and as a result needs to be written to disk if the "write" config flag is set. --- beets/importer.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index b8bbed65f..fdee257e9 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -45,7 +45,7 @@ from beets import mediafile action = Enum('action', ['SKIP', 'ASIS', 'TRACKS', 'MANUAL', 'APPLY', 'MANUAL_ID', - 'ALBUMS']) + 'ALBUMS', 'RETAG']) QUEUE_SIZE = 128 SINGLE_ARTIST_THRESH = 0.25 @@ -443,7 +443,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: @@ -482,7 +483,7 @@ class ImportTask(BaseImportTask): (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 +494,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() @@ -637,7 +638,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,8 +673,7 @@ class ImportTask(BaseImportTask): # old paths. item.move(copy, link) - # TODO: the EDIT_FLAG field is a hack! - if write and (self.apply or getattr(self, 'EDIT_FLAG', False)): + if write and (self.apply or self.choice_flag == action.RETAG): item.try_write() with session.lib.transaction(): @@ -1316,7 +1316,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( From b8ec22ca340b7c9205e812e2f098c15c15111dd8 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Mon, 1 Feb 2016 18:57:03 +0100 Subject: [PATCH 05/12] edit: use action.RETAG, cleanup * Make the edit plugin return action.RETAG when invoked during an interactive import session, making the importer handle the writing of the tags to the files (if needed) properly. * Move the logic relative to the "reference field" to _set_reference_field(), simplifying a bit the functions that depend on this field. * Hide the "edit Candidates" choice if no candidates are found. --- beetsplug/edit.py | 80 ++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 77984b77b..9961b6399 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -154,11 +154,21 @@ class EditPlugin(plugins.BeetsPlugin): }) self.register_listener('before_choose_candidate', - self.before_choose_candidate_event) + self.before_choose_candidate_listener) + self.register_listener('import_begin', self.import_begin_listener) - # Field to be used as "unequivocal, non-editable key" for an Item. - # TODO: cleanup - self.mapping_field = 'id' + def _set_reference_field(self, field): + """Set the "unequivocal, non-editable field" that will be used for + reconciling back the user changes. + """ + if field == 'id': + self.reference_field = 'id' + self.ref_field_value = lambda o: int(o.id) + self.obj_from_ref = lambda d: int(d['id']) + elif field == 'path': + self.reference_field = 'path' + self.ref_field_value = lambda o: util.displayable_path(o.path) + self.obj_from_ref = lambda d: util.displayable_path(d['path']) def commands(self): edit_command = ui.Subcommand( @@ -183,6 +193,9 @@ class EditPlugin(plugins.BeetsPlugin): def _edit_command(self, lib, opts, args): """The CLI command function for the `beet edit` command. """ + # Set the reference field to "id", as all Models have valid ids. + self._set_reference_field('id') + # Get the objects to edit. query = ui.decargs(args) items, albums = _do_query(lib, query, opts.album, False) @@ -211,8 +224,8 @@ class EditPlugin(plugins.BeetsPlugin): if extra: fields += extra - # Ensure we always have the mapping field for identification. - fields.append(self.mapping_field) + # Ensure we always have the reference field for identification. + fields.append(self.reference_field) return set(fields) @@ -274,15 +287,14 @@ class EditPlugin(plugins.BeetsPlugin): # If the objects are not on the DB yet, we need a copy of their # original state for show_model_changes. if all(not obj.id for obj in objs): - objs_old = deepcopy(objs) + objs_old = {self.ref_field_value(obj): deepcopy(obj) + for obj in objs} self.apply_data(objs, old_data, new_data) changed = False for obj in objs: if not obj.id: # TODO: remove uglyness - obj_old = next(x for x in objs_old if - getattr(x, self.mapping_field) == - getattr(obj, self.mapping_field)) + obj_old = objs_old[self.ref_field_value(obj)] else: obj_old = None changed |= ui.show_model_changes(obj, obj_old) @@ -315,24 +327,11 @@ class EditPlugin(plugins.BeetsPlugin): The objects are not written back to the database, so the changes are temporary. """ - # TODO: make this more pythonic - def ref_field_value(o): - if self.mapping_field == 'id': - return int(o.id) - elif self.mapping_field == 'path': - return util.displayable_path(o.path) - - def obj_from_ref(d): - if self.mapping_field == 'id': - return int(d['id']) - elif self.mapping_field == 'path': - return util.displayable_path(d['path']) - if len(old_data) != len(new_data): self._log.warn('number of objects changed from {} to {}', len(old_data), len(new_data)) - obj_by_f = {ref_field_value(o): o for o in objs} + obj_by_f = {self.ref_field_value(o): o for o in objs} ignore_fields = self.config['ignore_fields'].as_str_seq() for old_dict, new_dict in zip(old_data, new_data): # Prohibit any changes to forbidden fields to avoid @@ -346,8 +345,8 @@ class EditPlugin(plugins.BeetsPlugin): if forbidden: continue - # Reconcile back the user edits, using the mapping_field. - val = obj_from_ref(old_dict) + # Reconcile back the user edits, using the reference_field. + val = self.obj_from_ref(old_dict) apply(obj_by_f[val], new_dict) def save_changes(self, objs): @@ -361,32 +360,35 @@ class EditPlugin(plugins.BeetsPlugin): # Methods for interactive importer execution. - def before_choose_candidate_event(self, session, task): + def before_choose_candidate_listener(self, session, task): """Append an "Edit" choice to the interactive importer prompt. """ - return [PromptChoice('d', 'eDit', self.importer_edit), - PromptChoice('c', 'edit Candidates', - self.importer_edit_candidate)] + choices = [PromptChoice('d', 'eDit', self.importer_edit)] + if task.candidates: + choices.append(PromptChoice('c', 'edit Candidates', + self.importer_edit_candidate)) + + return choices + + def import_begin_listener(self, session): + """Initialize the reference field to 'path', as during an interactive + import session Models do not have valid 'id's yet. + """ + self._set_reference_field('path') def importer_edit(self, session, task): """Callback for invoking the functionality during an interactive import session on the *original* item tags. """ - # Make 'path' the mapping field, as the Items do not have ids yet. - # TODO: move to ~import_begin - self.mapping_field = 'path' - # Present the YAML to the user and let her change it. fields = self._get_fields(album=None, extra=[]) success = self.edit_objects(task.items, fields) # Save the new data. if success: - # TODO: implement properly, this is a quick illustrative hack. - # If using Items, the operation is something like - # "use the *modified* Items AS-IS *and* write() them" - task.EDIT_FLAG = True - return action.ASIS + # Return action.RETAG, which makes the importer write the tags + # to the files if needed. + return action.RETAG else: # Edit cancelled / no edits made. Revert changes. for obj in task.items: From 0e649e549a6292c8c0262e78806543ff137d82ee Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 3 Feb 2016 17:13:33 +0100 Subject: [PATCH 06/12] edit: add RETAG support for singleton, fix logic * Add support for the RETAG action to SingletonImportTask. * Modify ImportTask.align_album_level_fields() so the source of the information is a bit more intelligent in the RETAG case instead of always assuming the items come from applied metadata. --- beets/importer.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index fdee257e9..97cabb92b 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -621,8 +621,12 @@ class ImportTask(BaseImportTask): """Make some album fields equal across `self.items`. """ changes = {} + # Determine where to gather the info from for the RETAG action. + retag_asis = (self.choice_flag == action.RETAG and + not self.items[0].artist and + not self.items[0].mb_artistid) - if self.choice_flag == action.ASIS: + if self.choice_flag == action.ASIS or retag_asis: # Taking metadata "as-is". Guess whether this album is VA. plur_albumartist, freq = util.plurality( [i.albumartist or i.artist for i in self.items] @@ -808,8 +812,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) From 32f9bd507752d49bb7db89125a95ea2b8313dd1c Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 3 Feb 2016 17:17:48 +0100 Subject: [PATCH 07/12] edit: add tests for interactive importer execution * Add EditDuringImporterTest test case, covering the running of the plugin during an import session. Includes editing the "album" field and applying/discarding for both editing from items and editing from a candidate; and editing and applying for singletons for both editing from items and editing from a candidate. --- test/test_edit.py | 280 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 220 insertions(+), 60 deletions(-) diff --git a/test/test_edit.py b/test/test_edit.py index 25b24cea0..8a95c00b4 100644 --- a/test/test_edit.py +++ b/test/test_edit.py @@ -20,11 +20,15 @@ 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 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 +67,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 +85,63 @@ 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): + 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 +153,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 +166,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 +181,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 +194,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 +203,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 +251,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 +262,161 @@ 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): + 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__) From 0ba8f83929863daf03cc9aa1659c247e94f7482e Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 3 Feb 2016 17:22:03 +0100 Subject: [PATCH 08/12] edit: allow edit album+item fields in single yaml * Add support for editing both the item fields and the album fields in a single YAML file, by appending an Album-like object as the front of the objects to be edited. * The FakeAlbum class provides that object, mimicking the original Album behaviour and including an _apply_changes() method that propagates the changes read from the yaml onto the Items. * Modify edit_objects() so the flattening of the objects takes into account the type of object, using different fields for Albums and for Items. * Renamed apply() to apply_() to prevent an IDE warning about reusing a reserved built-in symbol. --- beetsplug/edit.py | 67 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 9 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 9961b6399..cc7c99632 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -21,7 +21,8 @@ from beets import plugins from beets import util from beets import ui from beets.dbcore import types -from beets.importer import action +from beets.importer import action, SingletonImportTask +from beets.library import Item, Album from beets.ui.commands import _do_query, PromptChoice from copy import deepcopy import subprocess @@ -121,7 +122,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. @@ -238,20 +239,26 @@ class EditPlugin(plugins.BeetsPlugin): everything). """ # Present the YAML to the user and let her change it. - success = self.edit_objects(objs, fields) + if album: + success = self.edit_objects(objs, None, fields) + else: + success = self.edit_objects(objs, fields, None) # Save the new data. if success: self.save_changes(objs) - def edit_objects(self, objs, fields): + def edit_objects(self, objs, item_fields, album_fields): """Dump a set of Model objects to a file as text, ask the user to edit it, and apply any changes to the objects. Return a boolean indicating whether the edit succeeded. """ # Get the content to edit as raw data structures. - old_data = [flatten(o, fields) for o in objs] + old_data = [flatten(o, + item_fields if isinstance(o, Item) + else album_fields) + for o in objs] # Set up a temporary file with the initial data for editing. new = NamedTemporaryFile(suffix='.yaml', delete=False) @@ -331,7 +338,7 @@ class EditPlugin(plugins.BeetsPlugin): self._log.warn('number of objects changed from {} to {}', len(old_data), len(new_data)) - obj_by_f = {self.ref_field_value(o): o for o in objs} + obj_by_ref = {self.ref_field_value(o): o for o in objs} ignore_fields = self.config['ignore_fields'].as_str_seq() for old_dict, new_dict in zip(old_data, new_data): # Prohibit any changes to forbidden fields to avoid @@ -347,7 +354,7 @@ class EditPlugin(plugins.BeetsPlugin): # Reconcile back the user edits, using the reference_field. val = self.obj_from_ref(old_dict) - apply(obj_by_f[val], new_dict) + apply_(obj_by_ref[val], new_dict) def save_changes(self, objs): """Save a list of updated Model objects to the database. @@ -380,12 +387,25 @@ class EditPlugin(plugins.BeetsPlugin): """Callback for invoking the functionality during an interactive import session on the *original* item tags. """ + singleton = isinstance(object, SingletonImportTask) + item_fields = self._get_fields(False, []) + items = list(task.items) # Shallow copy, not modifying task.items. + if not singleton: + # Prepend a FakeAlbum for allowing the user to edit album fields. + album = FakeAlbum(task.items, task.toppath) + items.insert(0, album) + album_fields = self._get_fields(True, []) + else: + album_fields = None + # Present the YAML to the user and let her change it. - fields = self._get_fields(album=None, extra=[]) - success = self.edit_objects(task.items, fields) + success = self.edit_objects(items, item_fields, album_fields) # Save the new data. if success: + if not singleton: + # Propagate the album changes to the items. + album._apply_changes() # Return action.RETAG, which makes the importer write the tags # to the files if needed. return action.RETAG @@ -405,3 +425,32 @@ class EditPlugin(plugins.BeetsPlugin): task.apply_metadata() return self.importer_edit(session, task) + + +class FakeAlbum(Album): + """Helper for presenting the user with an Album to be edited when there + is no real Album present. The album fields are set from the first item, + and after editing propagated to the items on `_apply_changes`. + """ + def __init__(self, items, path): + self._src_items = items + + # Create the album structure using metadata from the first item. + values = dict((key, items[0][key]) for key in Album.item_keys) + # Manually set the path as a single value field. + values[u'path'] = util.displayable_path(path) + super(FakeAlbum, self).__init__(**values) + + def _getters(self): + """Remove 'path' from Album._getters(), treating it as a regular field + in order to be able to use it directly.""" + getters = Album._getters() + getters.pop('path') + return getters + + def _apply_changes(self): + """Propagate changes to the album fields onto the Items. + """ + values = dict((key, self[key]) for key in Album.item_keys) + for i in self._src_items: + i.update(values) From 7b6c2c36d366af0792d6154e46c6e6369b760498 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 3 Feb 2016 18:58:29 +0100 Subject: [PATCH 09/12] edit: fix Plugin not unloaded during tests --- test/test_edit.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_edit.py b/test/test_edit.py index 8a95c00b4..c35c71c08 100644 --- a/test/test_edit.py +++ b/test/test_edit.py @@ -23,6 +23,7 @@ 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): @@ -126,6 +127,7 @@ class EditCommandTest(unittest.TestCase, TestHelper, EditMixin): self.mock_write = self.write_patcher.start() def tearDown(self): + EditPlugin.listeners = None self.write_patcher.stop() self.teardown_beets() self.unload_plugins() @@ -289,6 +291,7 @@ class EditDuringImporterTest(TerminalImportSessionSetup, unittest.TestCase, self.config['import']['timid'] = True def tearDown(self): + EditPlugin.listeners = None self.unload_plugins() self.teardown_beets() self.matcher.restore() From 8d3f9a573cbcfd9041df7735ba37bb08df464862 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 5 Feb 2016 12:35:11 +0100 Subject: [PATCH 10/12] edit: revert album-field changes, use temporary id * Revert the changes related to allowing the album- and item-level fields to be edited at the same time, as the increase in complexity was deemed excesive during review. * Modify the interactive execution so temporary Item.id's are used, removing the extra functionality needed for dealing with both id and path as reference fields. * Docstrings and comments cleanup. --- beetsplug/edit.py | 123 ++++++++++------------------------------------ 1 file changed, 27 insertions(+), 96 deletions(-) diff --git a/beetsplug/edit.py b/beetsplug/edit.py index cc7c99632..ee045664e 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -21,8 +21,7 @@ from beets import plugins from beets import util from beets import ui from beets.dbcore import types -from beets.importer import action, SingletonImportTask -from beets.library import Item, Album +from beets.importer import action from beets.ui.commands import _do_query, PromptChoice from copy import deepcopy import subprocess @@ -156,20 +155,6 @@ class EditPlugin(plugins.BeetsPlugin): self.register_listener('before_choose_candidate', self.before_choose_candidate_listener) - self.register_listener('import_begin', self.import_begin_listener) - - def _set_reference_field(self, field): - """Set the "unequivocal, non-editable field" that will be used for - reconciling back the user changes. - """ - if field == 'id': - self.reference_field = 'id' - self.ref_field_value = lambda o: int(o.id) - self.obj_from_ref = lambda d: int(d['id']) - elif field == 'path': - self.reference_field = 'path' - self.ref_field_value = lambda o: util.displayable_path(o.path) - self.obj_from_ref = lambda d: util.displayable_path(d['path']) def commands(self): edit_command = ui.Subcommand( @@ -194,9 +179,6 @@ class EditPlugin(plugins.BeetsPlugin): def _edit_command(self, lib, opts, args): """The CLI command function for the `beet edit` command. """ - # Set the reference field to "id", as all Models have valid ids. - self._set_reference_field('id') - # Get the objects to edit. query = ui.decargs(args) items, albums = _do_query(lib, query, opts.album, False) @@ -225,8 +207,8 @@ class EditPlugin(plugins.BeetsPlugin): if extra: fields += extra - # Ensure we always have the reference field for identification. - fields.append(self.reference_field) + # Ensure we always have the `id` field for identification. + fields.append('id') return set(fields) @@ -239,26 +221,20 @@ class EditPlugin(plugins.BeetsPlugin): everything). """ # Present the YAML to the user and let her change it. - if album: - success = self.edit_objects(objs, None, fields) - else: - success = self.edit_objects(objs, fields, None) + success = self.edit_objects(objs, fields) # Save the new data. if success: self.save_changes(objs) - def edit_objects(self, objs, item_fields, album_fields): + def edit_objects(self, objs, fields): """Dump a set of Model objects to a file as text, ask the user to edit it, and apply any changes to the objects. Return a boolean indicating whether the edit succeeded. """ # Get the content to edit as raw data structures. - old_data = [flatten(o, - item_fields if isinstance(o, Item) - else album_fields) - for o in objs] + old_data = [flatten(o, fields) for o in objs] # Set up a temporary file with the initial data for editing. new = NamedTemporaryFile(suffix='.yaml', delete=False) @@ -293,17 +269,11 @@ class EditPlugin(plugins.BeetsPlugin): # Show the changes. # If the objects are not on the DB yet, we need a copy of their # original state for show_model_changes. - if all(not obj.id for obj in objs): - objs_old = {self.ref_field_value(obj): deepcopy(obj) - for obj in objs} + 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: - if not obj.id: - # TODO: remove uglyness - obj_old = objs_old[self.ref_field_value(obj)] - else: - obj_old = None + 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.') @@ -338,7 +308,7 @@ class EditPlugin(plugins.BeetsPlugin): self._log.warn('number of objects changed from {} to {}', len(old_data), len(new_data)) - obj_by_ref = {self.ref_field_value(o): o for o in objs} + obj_by_id = {o.id: o for o in objs} ignore_fields = self.config['ignore_fields'].as_str_seq() for old_dict, new_dict in zip(old_data, new_data): # Prohibit any changes to forbidden fields to avoid @@ -352,9 +322,8 @@ class EditPlugin(plugins.BeetsPlugin): if forbidden: continue - # Reconcile back the user edits, using the reference_field. - val = self.obj_from_ref(old_dict) - apply_(obj_by_ref[val], 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. @@ -368,7 +337,8 @@ class EditPlugin(plugins.BeetsPlugin): # Methods for interactive importer execution. def before_choose_candidate_listener(self, session, task): - """Append an "Edit" choice to the interactive importer prompt. + """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: @@ -377,37 +347,26 @@ class EditPlugin(plugins.BeetsPlugin): return choices - def import_begin_listener(self, session): - """Initialize the reference field to 'path', as during an interactive - import session Models do not have valid 'id's yet. - """ - self._set_reference_field('path') - def importer_edit(self, session, task): """Callback for invoking the functionality during an interactive import session on the *original* item tags. """ - singleton = isinstance(object, SingletonImportTask) - item_fields = self._get_fields(False, []) - items = list(task.items) # Shallow copy, not modifying task.items. - if not singleton: - # Prepend a FakeAlbum for allowing the user to edit album fields. - album = FakeAlbum(task.items, task.toppath) - items.insert(0, album) - album_fields = self._get_fields(True, []) - else: - album_fields = None + # 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. - success = self.edit_objects(items, item_fields, album_fields) + 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: - if not singleton: - # Propagate the album changes to the items. - album._apply_changes() # Return action.RETAG, which makes the importer write the tags - # to the files if needed. + # to the files if needed without re-applying metadata. return action.RETAG else: # Edit cancelled / no edits made. Revert changes. @@ -416,41 +375,13 @@ class EditPlugin(plugins.BeetsPlugin): def importer_edit_candidate(self, session, task): """Callback for invoking the functionality during an interactive - import session on a *candidate* applied to the original items. + import session on a *candidate*. The candidate's metadata is + applied to the original items. """ - # Prompt the user for a candidate, and simulate matching. + # 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) - - -class FakeAlbum(Album): - """Helper for presenting the user with an Album to be edited when there - is no real Album present. The album fields are set from the first item, - and after editing propagated to the items on `_apply_changes`. - """ - def __init__(self, items, path): - self._src_items = items - - # Create the album structure using metadata from the first item. - values = dict((key, items[0][key]) for key in Album.item_keys) - # Manually set the path as a single value field. - values[u'path'] = util.displayable_path(path) - super(FakeAlbum, self).__init__(**values) - - def _getters(self): - """Remove 'path' from Album._getters(), treating it as a regular field - in order to be able to use it directly.""" - getters = Album._getters() - getters.pop('path') - return getters - - def _apply_changes(self): - """Propagate changes to the album fields onto the Items. - """ - values = dict((key, self[key]) for key in Album.item_keys) - for i in self._src_items: - i.update(values) From 7f75a066bd8fb8a05d2602c6f427fbe53b416fd1 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 5 Feb 2016 12:38:50 +0100 Subject: [PATCH 11/12] edit: RETAG importer fixes and docstrings * Remove the RETAG-specific logic on align_album_level_fields, assuming that the user will always make sure to have proper data on the first item. * Revise some docstrings and comments in order to clarify the use of RETAG. --- beets/importer.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 97cabb92b..f4dd4853d 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -46,6 +46,9 @@ from beets import mediafile action = Enum('action', ['SKIP', 'ASIS', 'TRACKS', 'MANUAL', 'APPLY', 'MANUAL_ID', '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 @@ -480,8 +483,8 @@ 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 in (action.ASIS, action.RETAG): return (self.cur_artist, self.cur_album) @@ -618,15 +621,14 @@ 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 = {} - # Determine where to gather the info from for the RETAG action. - retag_asis = (self.choice_flag == action.RETAG and - not self.items[0].artist and - not self.items[0].mb_artistid) - if self.choice_flag == action.ASIS or retag_asis: + if self.choice_flag == action.ASIS: # Taking metadata "as-is". Guess whether this album is VA. plur_albumartist, freq = util.plurality( [i.albumartist or i.artist for i in self.items] From 763813fdf71e59befdb6b05f783595b7639dfb15 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 5 Feb 2016 13:12:46 +0100 Subject: [PATCH 12/12] edit: add documentation * Add documentation to plugins/edit.rst about the execution of the plugin during the importer. --- docs/plugins/edit.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) 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 -------------