Merge pull request #1846 from diego-plan9/interactiveedit

edit: allow interactive editing during the importer
This commit is contained in:
Adrian Sampson 2016-02-07 14:34:54 -08:00
commit 6e3d72afa6
6 changed files with 341 additions and 83 deletions

View file

@ -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(

View file

@ -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.
"""

View file

@ -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)

View file

@ -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

View file

@ -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
-------------

View file

@ -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__)