From 3361aaafdaead7eaeac206decaf6c21331ef556d Mon Sep 17 00:00:00 2001 From: Aaron Date: Tue, 31 Jan 2017 02:15:29 -0800 Subject: [PATCH 01/18] =?UTF-8?q?Embedart=20plugin=20asks=20for=20confirma?= =?UTF-8?q?tion=20before=20making=20changes=20to=20item=E2=80=99s=20artwor?= =?UTF-8?q?k.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- beetsplug/embedart.py | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 948da6291..ff65e34db 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -20,7 +20,7 @@ import os.path from beets.plugins import BeetsPlugin from beets import ui -from beets.ui import decargs +from beets.ui import print_, decargs from beets.util import syspath, normpath, displayable_path, bytestring_path from beets.util.artresizer import ArtResizer from beets import config @@ -60,10 +60,28 @@ class EmbedCoverArtPlugin(BeetsPlugin): embed_cmd.parser.add_option( u'-f', u'--file', metavar='PATH', help=u'the image file to embed' ) + embed_cmd.parser.add_option( + u"-y", u"--yes", action="store_true", help=u"skip confirmation" + ) maxwidth = self.config['maxwidth'].get(int) compare_threshold = self.config['compare_threshold'].get(int) ifempty = self.config['ifempty'].get(bool) + def _confirmation(items, opts, fmt, prompt): + # Confirm artwork changes to library items. + if not opts.yes: + # Prepare confirmation with user. + print_() + + # Show all the items. + for item in items: + print_(format(item, fmt)) + + # Confirm with user. + if not ui.input_yn(prompt, True): + return False + return True + def embed_func(lib, opts, args): if opts.file: imagepath = normpath(opts.file) @@ -71,11 +89,30 @@ class EmbedCoverArtPlugin(BeetsPlugin): raise ui.UserError(u'image file {0} not found'.format( displayable_path(imagepath) )) - for item in lib.items(decargs(args)): + + items = lib.items(decargs(args)) + + # Confirm with user. + fmt = u'$albumartist - $album - $title' + prompt = u'Modify artwork for %i file%s (y/n)?' % \ + (len(items), 's' if len(items) > 1 else '') + if not _confirmation(items, opts, fmt, prompt): + return + + for item in items: art.embed_item(self._log, item, imagepath, maxwidth, None, compare_threshold, ifempty) else: - for album in lib.albums(decargs(args)): + items = lib.albums(decargs(args)) + + # Confirm with user. + fmt = u'$albumartist - $album' + prompt = u'Modify artwork for %i album%s (y/n)?' % \ + (len(items), 's' if len(items) > 1 else '') + if not _confirmation(items, opts, fmt, prompt): + return + + for album in items: art.embed_album(self._log, album, maxwidth, False, compare_threshold, ifempty) self.remove_artfile(album) From 9c97f95073c026114ef7d80b8a549ba7fed63452 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 1 Feb 2017 03:38:38 -0800 Subject: [PATCH 02/18] Updated embedart test cases to accomodate confirmation prompt. --- test/test_embedart.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/test_embedart.py b/test/test_embedart.py index ee08ecb4e..5360ea372 100644 --- a/test/test_embedart.py +++ b/test/test_embedart.py @@ -51,6 +51,9 @@ class EmbedartCliTest(_common.TestCase, TestHelper): abbey_differentpath = os.path.join(_common.RSRC, b'abbey-different.jpg') def setUp(self): + super(EmbedartCliTest, self).setUp() + self.io.install() + self.setup_beets() # Converter is threaded self.load_plugins('embedart') @@ -68,6 +71,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data() album = self.add_album_fixture() item = album.items()[0] + self.io.addinput('y') self.run_command('embedart', '-f', self.small_artpath) mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data) @@ -78,6 +82,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): item = album.items()[0] album.artpath = self.small_artpath album.store() + self.io.addinput('y') self.run_command('embedart') mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data) @@ -96,6 +101,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): album.store() config['embedart']['remove_art_file'] = True + self.io.addinput('y') self.run_command('embedart') if os.path.isfile(tmp_path): @@ -106,6 +112,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self.add_album_fixture() logging.getLogger('beets.embedart').setLevel(logging.DEBUG) with self.assertRaises(ui.UserError): + self.io.addinput('y') self.run_command('embedart', '-f', '/doesnotexist') def test_embed_non_image_file(self): @@ -117,6 +124,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): os.close(handle) try: + self.io.addinput('y') self.run_command('embedart', '-f', tmp_path) finally: os.remove(tmp_path) @@ -129,8 +137,10 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data(self.abbey_artpath) album = self.add_album_fixture() item = album.items()[0] + self.io.addinput('y') self.run_command('embedart', '-f', self.abbey_artpath) config['embedart']['compare_threshold'] = 20 + self.io.addinput('y') self.run_command('embedart', '-f', self.abbey_differentpath) mediafile = MediaFile(syspath(item.path)) @@ -143,9 +153,10 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data(self.abbey_similarpath) album = self.add_album_fixture() item = album.items()[0] + self.io.addinput('y') self.run_command('embedart', '-f', self.abbey_artpath) config['embedart']['compare_threshold'] = 20 - self.run_command('embedart', '-f', self.abbey_similarpath) + self.run_command('embedart', '-y', '-f', self.abbey_similarpath) mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data, @@ -169,7 +180,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): trackpath = album.items()[0].path albumpath = album.path shutil.copy(syspath(resource_path), syspath(trackpath)) - + self.run_command('extractart', '-n', 'extracted') self.assertExists(os.path.join(albumpath, b'extracted.jpg')) From d1ac8939155dab36b03a757c7e6c3b73fd35b634 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 1 Feb 2017 14:32:52 -0800 Subject: [PATCH 03/18] Style changes to pass PEP8 tests. --- beetsplug/embedart.py | 2 +- test/test_embedart.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index ff65e34db..056c8f36a 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -95,7 +95,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): # Confirm with user. fmt = u'$albumartist - $album - $title' prompt = u'Modify artwork for %i file%s (y/n)?' % \ - (len(items), 's' if len(items) > 1 else '') + (len(items), 's' if len(items) > 1 else '') if not _confirmation(items, opts, fmt, prompt): return diff --git a/test/test_embedart.py b/test/test_embedart.py index 5360ea372..b0e66dddb 100644 --- a/test/test_embedart.py +++ b/test/test_embedart.py @@ -180,7 +180,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): trackpath = album.items()[0].path albumpath = album.path shutil.copy(syspath(resource_path), syspath(trackpath)) - + self.run_command('extractart', '-n', 'extracted') self.assertExists(os.path.join(albumpath, b'extracted.jpg')) From c250a53b4d8073f0346c82eda66c1d47a9a86999 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 1 Feb 2017 17:04:01 -0800 Subject: [PATCH 04/18] Updated changelog and embedart docs. --- docs/changelog.rst | 2 ++ docs/plugins/embedart.rst | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0774722c8..0c3779c3c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,8 @@ New features: * :doc:`/plugins/badfiles`: Added a ``--verbose`` or ``-v`` option. Results are now displayed only for corrupted files by default and for all the files when the verbose option is set. :bug:`1654` :bug:`2434` + * :doc:`/plugins/embedart` by default now asks for confirmation before + embedding art into music files. Thanks to :user:`Stunner`. :bug:`1999` Fixes: diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 94e91d995..717bfb8e2 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -81,7 +81,8 @@ embedded album art: * ``beet embedart [-f IMAGE] QUERY``: embed images into the every track on the albums matching the query. If the ``-f`` (``--file``) option is given, then use a specific image file from the filesystem; otherwise, each album embeds - its own currently associated album art. + its own currently associated album art. Will prompt for confirmation before + making the change unless the ``-y`` (``--yes``) option is specified. * ``beet extractart [-a] [-n FILE] QUERY``: extracts the images for all albums matching the query. The images are placed inside the album folder. You can From 3e13971c545d376ec6ad555231f6dd9faee4eb8e Mon Sep 17 00:00:00 2001 From: Aaron Date: Mon, 13 Feb 2017 02:30:01 -0800 Subject: [PATCH 05/18] Some code cleanup/consolidation. --- beetsplug/embedart.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 056c8f36a..21fb0188c 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -67,12 +67,20 @@ class EmbedCoverArtPlugin(BeetsPlugin): compare_threshold = self.config['compare_threshold'].get(int) ifempty = self.config['ifempty'].get(bool) - def _confirmation(items, opts, fmt, prompt): + def _confirmation(items, opts): # Confirm artwork changes to library items. if not opts.yes: # Prepare confirmation with user. print_() + fmt = u'$albumartist - $album' + item_type = u'album' + if opts.file: + fmt = u'$albumartist - $album - $title' + item_type = u'file' + prompt = u'Modify artwork for %i %s%s (y/n)?' % \ + (len(items), item_type, 's' if len(items) > 1 else '') + # Show all the items. for item in items: print_(format(item, fmt)) @@ -93,10 +101,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): items = lib.items(decargs(args)) # Confirm with user. - fmt = u'$albumartist - $album - $title' - prompt = u'Modify artwork for %i file%s (y/n)?' % \ - (len(items), 's' if len(items) > 1 else '') - if not _confirmation(items, opts, fmt, prompt): + if not _confirmation(items, opts): return for item in items: @@ -106,10 +111,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): items = lib.albums(decargs(args)) # Confirm with user. - fmt = u'$albumartist - $album' - prompt = u'Modify artwork for %i album%s (y/n)?' % \ - (len(items), 's' if len(items) > 1 else '') - if not _confirmation(items, opts, fmt, prompt): + if not _confirmation(items, opts): return for album in items: From 733c1839fb3e67dc79f5ad707d993efc9676194d Mon Sep 17 00:00:00 2001 From: Aaron Date: Mon, 13 Feb 2017 02:43:13 -0800 Subject: [PATCH 06/18] Addressed coding style issue. --- beetsplug/embedart.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 21fb0188c..6344dc2b9 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -74,12 +74,12 @@ class EmbedCoverArtPlugin(BeetsPlugin): print_() fmt = u'$albumartist - $album' - item_type = u'album' + istr = u'album' if opts.file: fmt = u'$albumartist - $album - $title' - item_type = u'file' + istr = u'file' prompt = u'Modify artwork for %i %s%s (y/n)?' % \ - (len(items), item_type, 's' if len(items) > 1 else '') + (len(items), istr, 's' if len(items) > 1 else '') # Show all the items. for item in items: From 1fbbcf65fde067bd753907dc18218acc3f7dddee Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:29:09 -0500 Subject: [PATCH 07/18] Fix some confusing indentation --- beetsplug/embedart.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 6344dc2b9..f7e92e6f1 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -69,26 +69,26 @@ class EmbedCoverArtPlugin(BeetsPlugin): def _confirmation(items, opts): # Confirm artwork changes to library items. - if not opts.yes: - # Prepare confirmation with user. - print_() + if not opts.yes: + # Prepare confirmation with user. + print_() - fmt = u'$albumartist - $album' - istr = u'album' - if opts.file: - fmt = u'$albumartist - $album - $title' - istr = u'file' - prompt = u'Modify artwork for %i %s%s (y/n)?' % \ - (len(items), istr, 's' if len(items) > 1 else '') + fmt = u'$albumartist - $album' + istr = u'album' + if opts.file: + fmt = u'$albumartist - $album - $title' + istr = u'file' + prompt = u'Modify artwork for %i %s%s (y/n)?' % \ + (len(items), istr, 's' if len(items) > 1 else '') - # Show all the items. - for item in items: - print_(format(item, fmt)) + # Show all the items. + for item in items: + print_(format(item, fmt)) - # Confirm with user. - if not ui.input_yn(prompt, True): - return False - return True + # Confirm with user. + if not ui.input_yn(prompt, True): + return False + return True def embed_func(lib, opts, args): if opts.file: From c8499eb56d55b1f7d24ea26fbcba681ca427b92e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:34:53 -0500 Subject: [PATCH 08/18] _confirmation does not need to be a closure --- beetsplug/embedart.py | 50 +++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index f7e92e6f1..658b99599 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -27,6 +27,33 @@ from beets import config from beets import art +def _confirmation(items, opts): + """Show the list of affected objects (items or albums) and confirm + that the user wants to modify their artwork. + """ + # Confirm artwork changes to library items. + if not opts.yes: + # Prepare confirmation with user. + print_() + + fmt = u'$albumartist - $album' + istr = u'album' + if opts.file: + fmt = u'$albumartist - $album - $title' + istr = u'file' + prompt = u'Modify artwork for %i %s%s (y/n)?' % \ + (len(items), istr, 's' if len(items) > 1 else '') + + # Show all the items. + for item in items: + print_(format(item, fmt)) + + # Confirm with user. + if not ui.input_yn(prompt, True): + return False + return True + + class EmbedCoverArtPlugin(BeetsPlugin): """Allows albumart to be embedded into the actual files. """ @@ -67,29 +94,6 @@ class EmbedCoverArtPlugin(BeetsPlugin): compare_threshold = self.config['compare_threshold'].get(int) ifempty = self.config['ifempty'].get(bool) - def _confirmation(items, opts): - # Confirm artwork changes to library items. - if not opts.yes: - # Prepare confirmation with user. - print_() - - fmt = u'$albumartist - $album' - istr = u'album' - if opts.file: - fmt = u'$albumartist - $album - $title' - istr = u'file' - prompt = u'Modify artwork for %i %s%s (y/n)?' % \ - (len(items), istr, 's' if len(items) > 1 else '') - - # Show all the items. - for item in items: - print_(format(item, fmt)) - - # Confirm with user. - if not ui.input_yn(prompt, True): - return False - return True - def embed_func(lib, opts, args): if opts.file: imagepath = normpath(opts.file) From 430595825f624e00cf7cb4a86bd8568fd02257a3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:37:04 -0500 Subject: [PATCH 09/18] Rename parameter to be more sensible We don't just take items; they're either items or albums. --- beetsplug/embedart.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 658b99599..c3463c1a6 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -27,7 +27,7 @@ from beets import config from beets import art -def _confirmation(items, opts): +def _confirmation(objs, opts): """Show the list of affected objects (items or albums) and confirm that the user wants to modify their artwork. """ @@ -42,11 +42,11 @@ def _confirmation(items, opts): fmt = u'$albumartist - $album - $title' istr = u'file' prompt = u'Modify artwork for %i %s%s (y/n)?' % \ - (len(items), istr, 's' if len(items) > 1 else '') + (len(objs), istr, 's' if len(objs) > 1 else '') - # Show all the items. - for item in items: - print_(format(item, fmt)) + # Show all the items or albums. + for obj in objs: + print_(format(obj, fmt)) # Confirm with user. if not ui.input_yn(prompt, True): From eddbab63bd5e4efaaeeab42220771a5e9387b1fe Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:37:33 -0500 Subject: [PATCH 10/18] Remove extraneous blank line --- beetsplug/embedart.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index c3463c1a6..39cab585e 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -33,9 +33,6 @@ def _confirmation(objs, opts): """ # Confirm artwork changes to library items. if not opts.yes: - # Prepare confirmation with user. - print_() - fmt = u'$albumartist - $album' istr = u'album' if opts.file: From 3a436bb33dc5553c3d447a5d77a650a5b248466a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:38:16 -0500 Subject: [PATCH 11/18] Remove explicit format We now use the default formats from the configuration. --- beetsplug/embedart.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 39cab585e..618761e90 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -33,17 +33,15 @@ def _confirmation(objs, opts): """ # Confirm artwork changes to library items. if not opts.yes: - fmt = u'$albumartist - $album' istr = u'album' if opts.file: - fmt = u'$albumartist - $album - $title' istr = u'file' prompt = u'Modify artwork for %i %s%s (y/n)?' % \ (len(objs), istr, 's' if len(objs) > 1 else '') # Show all the items or albums. for obj in objs: - print_(format(obj, fmt)) + print_(format(obj)) # Confirm with user. if not ui.input_yn(prompt, True): From f985970b252346f40dc22fdb6ef678442b966eb0 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:42:20 -0500 Subject: [PATCH 12/18] Simplify scope of _confirmation and formatting --- beetsplug/embedart.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 618761e90..4f6cf1580 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -27,26 +27,26 @@ from beets import config from beets import art -def _confirmation(objs, opts): +def _confirmation(objs, album): """Show the list of affected objects (items or albums) and confirm that the user wants to modify their artwork. + + `album` is a Boolean indicating whether these are albums (as opposed + to items). """ - # Confirm artwork changes to library items. - if not opts.yes: - istr = u'album' - if opts.file: - istr = u'file' - prompt = u'Modify artwork for %i %s%s (y/n)?' % \ - (len(objs), istr, 's' if len(objs) > 1 else '') + noun = u'album' if album else u'file' + prompt = u'Modify artwork for {} {}{} (y/n)?'.format( + len(objs), + noun, + u's' if len(objs) > 1 else u'' + ) - # Show all the items or albums. - for obj in objs: - print_(format(obj)) + # Show all the items or albums. + for obj in objs: + print_(format(obj)) - # Confirm with user. - if not ui.input_yn(prompt, True): - return False - return True + # Confirm with user. + return ui.input_yn(prompt, True) class EmbedCoverArtPlugin(BeetsPlugin): @@ -100,7 +100,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): items = lib.items(decargs(args)) # Confirm with user. - if not _confirmation(items, opts): + if not opts.yes and not _confirmation(items, not opts.file): return for item in items: @@ -110,7 +110,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): items = lib.albums(decargs(args)) # Confirm with user. - if not _confirmation(items, opts): + if not opts.yes and not _confirmation(items, not opts.file): return for album in items: From f0f55d11ec535ad3159336d19cdd51c937759ca9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:42:38 -0500 Subject: [PATCH 13/18] Rename _confirmation to _confirm --- beetsplug/embedart.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 4f6cf1580..3d7f4f993 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -27,7 +27,7 @@ from beets import config from beets import art -def _confirmation(objs, album): +def _confirm(objs, album): """Show the list of affected objects (items or albums) and confirm that the user wants to modify their artwork. @@ -100,20 +100,20 @@ class EmbedCoverArtPlugin(BeetsPlugin): items = lib.items(decargs(args)) # Confirm with user. - if not opts.yes and not _confirmation(items, not opts.file): + if not opts.yes and not _confirm(items, not opts.file): return for item in items: art.embed_item(self._log, item, imagepath, maxwidth, None, compare_threshold, ifempty) else: - items = lib.albums(decargs(args)) + albums = lib.albums(decargs(args)) # Confirm with user. - if not opts.yes and not _confirmation(items, not opts.file): + if not opts.yes and not _confirm(albums, not opts.file): return - for album in items: + for album in albums: art.embed_album(self._log, album, maxwidth, False, compare_threshold, ifempty) self.remove_artfile(album) From 8ce7b49ed812f4fa704f822c90a6f526dbf2d446 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:45:29 -0500 Subject: [PATCH 14/18] Default to confirm --- beetsplug/embedart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 3d7f4f993..51d401af8 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -35,7 +35,7 @@ def _confirm(objs, album): to items). """ noun = u'album' if album else u'file' - prompt = u'Modify artwork for {} {}{} (y/n)?'.format( + prompt = u'Modify artwork for {} {}{} (Y/n)?'.format( len(objs), noun, u's' if len(objs) > 1 else u'' @@ -46,7 +46,7 @@ def _confirm(objs, album): print_(format(obj)) # Confirm with user. - return ui.input_yn(prompt, True) + return ui.input_yn(prompt) class EmbedCoverArtPlugin(BeetsPlugin): From b91185a9ff7cd7aecd65bbda10e7971921f7b2b3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:53:17 -0500 Subject: [PATCH 15/18] Docs tweaks for #2422 --- docs/changelog.rst | 5 +++-- docs/plugins/embedart.rst | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index fd37f992e..dd597d933 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -31,8 +31,9 @@ New features: * A new :ref:`hardlink` config option instructs the importer to create hard links on filesystems that support them. Thanks to :user:`jacobwgillespie`. :bug:`2445` -* :doc:`/plugins/embedart` by default now asks for confirmation before - embedding art into music files. Thanks to :user:`Stunner`. :bug:`1999` +* :doc:`/plugins/embedart`: The explicit ``embedart`` command now asks for + confirmation before embedding art into music files. Thanks to + :user:`Stunner`. :bug:`1999` * You can now run beets by typing `python -m beets`. :bug:`2453` * A new :doc:`/plugins/kodiupdate` lets you keep your Kodi library in sync with beets. Thanks to :user:`Pauligrinder`. :bug:`2411` diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 717bfb8e2..68ea0f664 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -81,8 +81,8 @@ embedded album art: * ``beet embedart [-f IMAGE] QUERY``: embed images into the every track on the albums matching the query. If the ``-f`` (``--file``) option is given, then use a specific image file from the filesystem; otherwise, each album embeds - its own currently associated album art. Will prompt for confirmation before - making the change unless the ``-y`` (``--yes``) option is specified. + its own currently associated album art. The command prompts for confirmation + before making the change unless you specify the ``-y`` (``--yes``) option. * ``beet extractart [-a] [-n FILE] QUERY``: extracts the images for all albums matching the query. The images are placed inside the album folder. You can From 5a71ce722a672431c035ad3b6c430b97fbaacdc9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 6 Mar 2017 23:55:14 -0500 Subject: [PATCH 16/18] Simplify embedart test changes for #2422 Whenever possible, it's nice to avoid using DummyIO---it can make debugging difficult. --- test/test_embedart.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/test/test_embedart.py b/test/test_embedart.py index b0e66dddb..ad04c57d7 100644 --- a/test/test_embedart.py +++ b/test/test_embedart.py @@ -52,8 +52,6 @@ class EmbedartCliTest(_common.TestCase, TestHelper): def setUp(self): super(EmbedartCliTest, self).setUp() - self.io.install() - self.setup_beets() # Converter is threaded self.load_plugins('embedart') @@ -71,8 +69,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data() album = self.add_album_fixture() item = album.items()[0] - self.io.addinput('y') - self.run_command('embedart', '-f', self.small_artpath) + self.run_command('embedart', '-y', '-f', self.small_artpath) mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data) @@ -82,8 +79,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): item = album.items()[0] album.artpath = self.small_artpath album.store() - self.io.addinput('y') - self.run_command('embedart') + self.run_command('embedart', '-y') mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data) @@ -101,8 +97,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): album.store() config['embedart']['remove_art_file'] = True - self.io.addinput('y') - self.run_command('embedart') + self.run_command('embedart', '-y') if os.path.isfile(tmp_path): os.remove(tmp_path) @@ -112,8 +107,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self.add_album_fixture() logging.getLogger('beets.embedart').setLevel(logging.DEBUG) with self.assertRaises(ui.UserError): - self.io.addinput('y') - self.run_command('embedart', '-f', '/doesnotexist') + self.run_command('embedart', '-y', '-f', '/doesnotexist') def test_embed_non_image_file(self): album = self.add_album_fixture() @@ -124,8 +118,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): os.close(handle) try: - self.io.addinput('y') - self.run_command('embedart', '-f', tmp_path) + self.run_command('embedart', '-y', '-f', tmp_path) finally: os.remove(tmp_path) @@ -137,11 +130,9 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data(self.abbey_artpath) album = self.add_album_fixture() item = album.items()[0] - self.io.addinput('y') - self.run_command('embedart', '-f', self.abbey_artpath) + self.run_command('embedart', '-y', '-f', self.abbey_artpath) config['embedart']['compare_threshold'] = 20 - self.io.addinput('y') - self.run_command('embedart', '-f', self.abbey_differentpath) + self.run_command('embedart', '-y', '-f', self.abbey_differentpath) mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data, @@ -153,8 +144,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self._setup_data(self.abbey_similarpath) album = self.add_album_fixture() item = album.items()[0] - self.io.addinput('y') - self.run_command('embedart', '-f', self.abbey_artpath) + self.run_command('embedart', '-y', '-f', self.abbey_artpath) config['embedart']['compare_threshold'] = 20 self.run_command('embedart', '-y', '-f', self.abbey_similarpath) mediafile = MediaFile(syspath(item.path)) From f6df3befac07109848594af46459853fd12b51b3 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 8 Mar 2017 19:06:09 -0800 Subject: [PATCH 17/18] Added interactive test method for embedart plugin. --- test/test_embedart.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test_embedart.py b/test/test_embedart.py index ad04c57d7..30e4841ba 100644 --- a/test/test_embedart.py +++ b/test/test_embedart.py @@ -52,6 +52,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): def setUp(self): super(EmbedartCliTest, self).setUp() + self.io.install() self.setup_beets() # Converter is threaded self.load_plugins('embedart') @@ -65,6 +66,15 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() + def test_embed_art_from_file_with_input(self): + self._setup_data() + album = self.add_album_fixture() + item = album.items()[0] + self.io.addinput('y') + self.run_command('embedart', '-f', self.small_artpath) + mediafile = MediaFile(syspath(item.path)) + self.assertEqual(mediafile.images[0].data, self.image_data) + def test_embed_art_from_file(self): self._setup_data() album = self.add_album_fixture() From 64d69f0817c0282fa71ee980ce3c6d602ce52730 Mon Sep 17 00:00:00 2001 From: Aaron Date: Fri, 10 Mar 2017 23:30:49 -0800 Subject: [PATCH 18/18] =?UTF-8?q?embedart:=20Added=20test=20case=20for=20i?= =?UTF-8?q?nputting=20=E2=80=9Cno=E2=80=9D=20option=20interactively.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/test_embedart.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/test_embedart.py b/test/test_embedart.py index 30e4841ba..1622fffb4 100644 --- a/test/test_embedart.py +++ b/test/test_embedart.py @@ -66,7 +66,7 @@ class EmbedartCliTest(_common.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() - def test_embed_art_from_file_with_input(self): + def test_embed_art_from_file_with_yes_input(self): self._setup_data() album = self.add_album_fixture() item = album.items()[0] @@ -75,6 +75,16 @@ class EmbedartCliTest(_common.TestCase, TestHelper): mediafile = MediaFile(syspath(item.path)) self.assertEqual(mediafile.images[0].data, self.image_data) + def test_embed_art_from_file_with_no_input(self): + self._setup_data() + album = self.add_album_fixture() + item = album.items()[0] + self.io.addinput('n') + self.run_command('embedart', '-f', self.small_artpath) + mediafile = MediaFile(syspath(item.path)) + # make sure that images array is empty (nothing embedded) + self.assertEqual(len(mediafile.images), 0) + def test_embed_art_from_file(self): self._setup_data() album = self.add_album_fixture()