From 4d4113e3a4a157c60e81b4075fec7310547880e3 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:15:09 -0500 Subject: [PATCH 01/64] Refactor ftintitle to extract the code to find the featured artist This removes the code that extracts the featured artist from the original artist field from the ft_in_title function and puts it into its own. It also adds a custom ArtistNotFoundException that find_feat_part will throw if the album artist is not in the artist field. This allows us to easily test the results of finding a featured artist in the artist sting, and thus adds tests for the usual test cases that the split_on_feat gets tested for. --- beetsplug/ftintitle.py | 56 ++++++++++++++++++++++++++++-------------- test/test_ftintitle.py | 15 +++++++++++ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index f28a1661c..bfeaf734d 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -23,6 +23,11 @@ import re log = logging.getLogger('beets') +class ArtistNotFoundException(Exception): + def __init__(self, value): + self.value = value + def __str__(self): + return repr(self.value) def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist @@ -67,6 +72,34 @@ def update_metadata(item, feat_part, drop_feat, loglevel=logging.DEBUG): item.title = new_title +def find_feat_part(artist, albumartist): + """Attempt to find featured artists in the item's artist fields and + return the results. Returns None if no featured artist found. + """ + feat_part = None + + # Look for the album artist in the artist field. If it's not + # present, give up. + albumartist_split = artist.split(albumartist, 1) + if len(albumartist_split) <= 1: + raise ArtistNotFoundException('album artist not present in artist') + + # If the last element of the split (the right-hand side of the + # album artist) is nonempty, then it probably contains the + # featured artist. + elif albumartist_split[-1] != '': + # Extract the featured artist from the right-hand side. + _, feat_part = split_on_feat(albumartist_split[-1]) + + # Otherwise, if there's nothing on the right-hand side, look for a + # featuring artist on the left-hand side. + else: + lhs, rhs = split_on_feat(albumartist_split[0]) + if rhs: + feat_part = lhs + + return feat_part + def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): """Look for featured artists in the item's artist fields and move them to the title. @@ -80,28 +113,13 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): _, featured = split_on_feat(artist) if featured and albumartist != artist and albumartist: log.log(loglevel, displayable_path(item.path)) - feat_part = None - # Look for the album artist in the artist field. If it's not - # present, give up. - albumartist_split = artist.split(albumartist, 1) - if len(albumartist_split) <= 1: + # Attempt to find the featured artist + try: + feat_part = find_feat_part(artist, albumartist) + except ArtistNotFoundException: log.log(loglevel, 'album artist not present in artist') - # If the last element of the split (the right-hand side of the - # album artist) is nonempty, then it probably contains the - # featured artist. - elif albumartist_split[-1] != '': - # Extract the featured artist from the right-hand side. - _, feat_part = split_on_feat(albumartist_split[-1]) - - # Otherwise, if there's nothing on the right-hand side, look for a - # featuring artist on the left-hand side. - else: - lhs, rhs = split_on_feat(albumartist_split[0]) - if rhs: - feat_part = lhs - # If we have a featuring artist, move it to the title. if feat_part: update_metadata(item, feat_part, drop_feat, loglevel) diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index 77e416c5a..cffa3332a 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -23,6 +23,21 @@ class FtInTitlePluginTest(unittest.TestCase): """Set up configuration""" ftintitle.FtInTitlePlugin() + def test_find_feat_part(self): + test_cases = [ + {'artist': 'Alice ft. Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice feat Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice featuring Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice & Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, + ] + + for test_case in test_cases: + feat_part = ftintitle.find_feat_part(test_case['artist'], test_case['album_artist']) + self.assertEqual(feat_part, test_case['feat_part']) + def test_split_on_feat(self): parts = ftintitle.split_on_feat('Alice ft. Bob') self.assertEqual(parts, ('Alice', 'Bob')) From a70820d8a1fc2d16fac2cd89b73a3b86d85ea931 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:20:29 -0500 Subject: [PATCH 02/64] Fix a bug in ftintitle with the order of album artist There was a bug in the find_feat_part function that would cause it to fail if the album artist was the second part of the featured string. For example, if the Artist field was Alice & Bob, and the Album Artist field was Bob it would return None due to the order. This fixes that and adds test cases to ensure it doesn't return. --- beetsplug/ftintitle.py | 2 +- test/test_ftintitle.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index bfeaf734d..d332b0c08 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -95,7 +95,7 @@ def find_feat_part(artist, albumartist): # featuring artist on the left-hand side. else: lhs, rhs = split_on_feat(albumartist_split[0]) - if rhs: + if lhs: feat_part = lhs return feat_part diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index cffa3332a..fa51dfc14 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -32,6 +32,8 @@ class FtInTitlePluginTest(unittest.TestCase): {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, + {'artist': 'Alice & Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, + {'artist': 'Alice ft. Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, ] for test_case in test_cases: From 8f4181843445023fd9fb20426a63d084f59ceccb Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:58:59 -0500 Subject: [PATCH 03/64] Fix formatting to pass flake8 tests --- beetsplug/ftintitle.py | 4 +++ test/test_ftintitle.py | 59 +++++++++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index d332b0c08..fbeb7bc27 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -23,12 +23,15 @@ import re log = logging.getLogger('beets') + class ArtistNotFoundException(Exception): def __init__(self, value): self.value = value + def __str__(self): return repr(self.value) + def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main @@ -100,6 +103,7 @@ def find_feat_part(artist, albumartist): return feat_part + def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): """Look for featured artists in the item's artist fields and move them to the title. diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index fa51dfc14..249569566 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -25,19 +25,58 @@ class FtInTitlePluginTest(unittest.TestCase): def test_find_feat_part(self): test_cases = [ - {'artist': 'Alice ft. Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice feat Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice featuring Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice & Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, - {'artist': 'Alice & Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, - {'artist': 'Alice ft. Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, + { + 'artist': 'Alice ft. Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice feat Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice featuring Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice & Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice and Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice With Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice defeat Bob', + 'album_artist': 'Alice', + 'feat_part': None + }, + { + 'artist': 'Alice & Bob', + 'album_artist': 'Bob', + 'feat_part': 'Alice' + }, + { + 'artist': 'Alice ft. Bob', + 'album_artist': 'Bob', + 'feat_part': 'Alice' + }, ] for test_case in test_cases: - feat_part = ftintitle.find_feat_part(test_case['artist'], test_case['album_artist']) + feat_part = ftintitle.find_feat_part( + test_case['artist'], + test_case['album_artist'] + ) self.assertEqual(feat_part, test_case['feat_part']) def test_split_on_feat(self): From e71c464c141c51e65807d0ca19e50d28caf5776e Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 21:27:58 -0500 Subject: [PATCH 04/64] Initialize feat_part as None --- beetsplug/ftintitle.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index fbeb7bc27..47d918f5f 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -118,6 +118,8 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): if featured and albumartist != artist and albumartist: log.log(loglevel, displayable_path(item.path)) + feat_part = None + # Attempt to find the featured artist try: feat_part = find_feat_part(artist, albumartist) From 8cd3d0059fe7fbe655c346cd801d9fe3acbd1612 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Mon, 29 Dec 2014 11:10:43 -0500 Subject: [PATCH 05/64] Remove ArtistNotFoundException in favor of returning None for simplicity --- beetsplug/ftintitle.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 47d918f5f..c241e1f08 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -24,14 +24,6 @@ import re log = logging.getLogger('beets') -class ArtistNotFoundException(Exception): - def __init__(self, value): - self.value = value - - def __str__(self): - return repr(self.value) - - def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main @@ -85,7 +77,7 @@ def find_feat_part(artist, albumartist): # present, give up. albumartist_split = artist.split(albumartist, 1) if len(albumartist_split) <= 1: - raise ArtistNotFoundException('album artist not present in artist') + return feat_part # If the last element of the split (the right-hand side of the # album artist) is nonempty, then it probably contains the @@ -121,10 +113,7 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): feat_part = None # Attempt to find the featured artist - try: - feat_part = find_feat_part(artist, albumartist) - except ArtistNotFoundException: - log.log(loglevel, 'album artist not present in artist') + feat_part = find_feat_part(artist, albumartist) # If we have a featuring artist, move it to the title. if feat_part: From b1bf7f3e681cec8f9aa01c651dff239f3e1fbfcc Mon Sep 17 00:00:00 2001 From: mried Date: Sat, 24 Jan 2015 11:15:36 +0100 Subject: [PATCH 06/64] Added an option to extract the art file of all matched albums. Closes #1261 --- beetsplug/embedart.py | 20 +++++++++++++++++--- docs/plugins/embedart.rst | 6 ++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 4609f9f50..d98fc1328 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -86,12 +86,26 @@ class EmbedCoverArtPlugin(BeetsPlugin): help='extract an image from file metadata') extract_cmd.parser.add_option('-o', dest='outpath', help='image output file') + extract_cmd.parser.add_option('-a', dest='albums', action='store_true', + help='extract the art of all matching albums') def extract_func(lib, opts, args): - outpath = normpath(opts.outpath or config['art_filename'].get()) - for item in lib.items(decargs(args)): - if self.extract(outpath, item): + if opts.albums: + if opts.outpath and os.path.sep in normpath(opts.outpath): + self._log.error(u"When using -a, only specify a filename instead" + u" of a full path for -o") return + for album in lib.albums(decargs(args)): + outpath = normpath(os.path.join(album.path, opts.outpath + or config['art_filename'].get())) + for item in album.items(): + if self.extract(outpath, item): + return + else: + outpath = normpath(opts.outpath or config['art_filename'].get()) + for item in lib.items(decargs(args)): + if self.extract(outpath, item): + return extract_cmd.func = extract_func # Clear command. diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 273046979..2c8748d14 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -84,5 +84,11 @@ embedded album art: ``art_filename`` configuration option. It defaults to ``cover`` if it's not specified via ``-o`` nor the config. +* ``beet extractart -a [-o FILE] QUERY``: extracts the images for all albums + matching the query. The images are placed inside the album folder. The + destination filename is taken either from the ``-o`` option or the + ``art_filename`` configuration option. It defaults to ``cover`` if it's not + specified. + * ``beet clearart QUERY``: removes all embedded images from all items matching the query. (Use with caution!) From de7768deae68a0ea0932d01de77d30c172f79a9a Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sat, 24 Jan 2015 15:59:13 +0100 Subject: [PATCH 07/64] Bugfixes and code rearrange for the extract art for albums feature. Closes #1261 --- beetsplug/embedart.py | 29 +++++++++++++++-------------- docs/changelog.rst | 2 ++ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index d98fc1328..6b87b5a92 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -87,25 +87,22 @@ class EmbedCoverArtPlugin(BeetsPlugin): extract_cmd.parser.add_option('-o', dest='outpath', help='image output file') extract_cmd.parser.add_option('-a', dest='albums', action='store_true', - help='extract the art of all matching albums') + help='extract the art of all matching ' + 'albums') def extract_func(lib, opts, args): + outpath = opts.outpath or config['art_filename'].get() if opts.albums: - if opts.outpath and os.path.sep in normpath(opts.outpath): - self._log.error(u"When using -a, only specify a filename instead" - u" of a full path for -o") + if opts.outpath and '/' in opts.outpath.replace('\\', '/'): + self._log.error(u"When using -a, only specify a filename " + u"instead of a full path for -o") return for album in lib.albums(decargs(args)): - outpath = normpath(os.path.join(album.path, opts.outpath - or config['art_filename'].get())) - for item in album.items(): - if self.extract(outpath, item): - return + artpath = normpath(os.path.join(album.path, outpath)) + self.extract_first(artpath, album.items()) else: - outpath = normpath(opts.outpath or config['art_filename'].get()) - for item in lib.items(decargs(args)): - if self.extract(outpath, item): - return + outpath = normpath(outpath) + self.extract_first(outpath, lib.items(decargs(args))) extract_cmd.func = extract_func # Clear command. @@ -258,7 +255,6 @@ class EmbedCoverArtPlugin(BeetsPlugin): return mf.art # 'extractart' command. - def extract(self, outpath, item): if not item: self._log.error(u'No item matches query.') @@ -284,6 +280,11 @@ class EmbedCoverArtPlugin(BeetsPlugin): f.write(art) return outpath + def extract_first(self, outpath, items): + for item in items: + if self.extract(outpath, item): + return outpath + # 'clearart' command. def clear(self, lib, query): self._log.info(u'Clearing album art from items:') diff --git a/docs/changelog.rst b/docs/changelog.rst index 7ad300504..0774de98c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ Features: by default. :bug:`1246` * :doc:`/plugins/fetchart`: Names of extracted image art is taken from the ``art_filename`` configuration option. :bug:`1258` +* :doc:`/plugins/fetchart`: New option ``-a`` to extract the cover art of all + matched albums into its directory. :bug:`1261` Fixes: From 6ac132edf75ad562cc95ec722ed790cbcfbb1c93 Mon Sep 17 00:00:00 2001 From: mried Date: Sat, 24 Jan 2015 11:15:36 +0100 Subject: [PATCH 08/64] Added an option to extract the art file of all matched albums. Closes #1261 --- beetsplug/embedart.py | 20 +++++++++++++++++--- docs/plugins/embedart.rst | 6 ++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 4609f9f50..d98fc1328 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -86,12 +86,26 @@ class EmbedCoverArtPlugin(BeetsPlugin): help='extract an image from file metadata') extract_cmd.parser.add_option('-o', dest='outpath', help='image output file') + extract_cmd.parser.add_option('-a', dest='albums', action='store_true', + help='extract the art of all matching albums') def extract_func(lib, opts, args): - outpath = normpath(opts.outpath or config['art_filename'].get()) - for item in lib.items(decargs(args)): - if self.extract(outpath, item): + if opts.albums: + if opts.outpath and os.path.sep in normpath(opts.outpath): + self._log.error(u"When using -a, only specify a filename instead" + u" of a full path for -o") return + for album in lib.albums(decargs(args)): + outpath = normpath(os.path.join(album.path, opts.outpath + or config['art_filename'].get())) + for item in album.items(): + if self.extract(outpath, item): + return + else: + outpath = normpath(opts.outpath or config['art_filename'].get()) + for item in lib.items(decargs(args)): + if self.extract(outpath, item): + return extract_cmd.func = extract_func # Clear command. diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 273046979..2c8748d14 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -84,5 +84,11 @@ embedded album art: ``art_filename`` configuration option. It defaults to ``cover`` if it's not specified via ``-o`` nor the config. +* ``beet extractart -a [-o FILE] QUERY``: extracts the images for all albums + matching the query. The images are placed inside the album folder. The + destination filename is taken either from the ``-o`` option or the + ``art_filename`` configuration option. It defaults to ``cover`` if it's not + specified. + * ``beet clearart QUERY``: removes all embedded images from all items matching the query. (Use with caution!) From c43173263c2949f57a82d1c41267bf9b69da2352 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sat, 24 Jan 2015 15:59:13 +0100 Subject: [PATCH 09/64] Bugfixes and code rearrange for the extract art for albums feature. Closes #1261 --- beetsplug/embedart.py | 29 +++++++++++++++-------------- docs/changelog.rst | 2 ++ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index d98fc1328..6b87b5a92 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -87,25 +87,22 @@ class EmbedCoverArtPlugin(BeetsPlugin): extract_cmd.parser.add_option('-o', dest='outpath', help='image output file') extract_cmd.parser.add_option('-a', dest='albums', action='store_true', - help='extract the art of all matching albums') + help='extract the art of all matching ' + 'albums') def extract_func(lib, opts, args): + outpath = opts.outpath or config['art_filename'].get() if opts.albums: - if opts.outpath and os.path.sep in normpath(opts.outpath): - self._log.error(u"When using -a, only specify a filename instead" - u" of a full path for -o") + if opts.outpath and '/' in opts.outpath.replace('\\', '/'): + self._log.error(u"When using -a, only specify a filename " + u"instead of a full path for -o") return for album in lib.albums(decargs(args)): - outpath = normpath(os.path.join(album.path, opts.outpath - or config['art_filename'].get())) - for item in album.items(): - if self.extract(outpath, item): - return + artpath = normpath(os.path.join(album.path, outpath)) + self.extract_first(artpath, album.items()) else: - outpath = normpath(opts.outpath or config['art_filename'].get()) - for item in lib.items(decargs(args)): - if self.extract(outpath, item): - return + outpath = normpath(outpath) + self.extract_first(outpath, lib.items(decargs(args))) extract_cmd.func = extract_func # Clear command. @@ -258,7 +255,6 @@ class EmbedCoverArtPlugin(BeetsPlugin): return mf.art # 'extractart' command. - def extract(self, outpath, item): if not item: self._log.error(u'No item matches query.') @@ -284,6 +280,11 @@ class EmbedCoverArtPlugin(BeetsPlugin): f.write(art) return outpath + def extract_first(self, outpath, items): + for item in items: + if self.extract(outpath, item): + return outpath + # 'clearart' command. def clear(self, lib, query): self._log.info(u'Clearing album art from items:') diff --git a/docs/changelog.rst b/docs/changelog.rst index 96264ef6e..6c4295ae8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ Features: by default. :bug:`1246` * :doc:`/plugins/fetchart`: Names of extracted image art is taken from the ``art_filename`` configuration option. :bug:`1258` +* :doc:`/plugins/fetchart`: New option ``-a`` to extract the cover art of all + matched albums into its directory. :bug:`1261` Fixes: From 67e106522928b97f8e85e8e1044ff2bae13381dc Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Mon, 26 Jan 2015 14:25:32 +0100 Subject: [PATCH 10/64] Display clear error message when MusicBrainz is unreachable: "MusicBrainz not reachable" Catch musicbrainzngs.WebServiceError before the general musicbrainzngs.MusicBrainzError Fix #1190 --- beets/autotag/mb.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index 6889eeaf6..50d146dbb 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -325,6 +325,11 @@ def match_album(artist, album, tracks=None): try: res = musicbrainzngs.search_releases( limit=config['musicbrainz']['searchlimit'].get(int), **criteria) + except musicbrainzngs.WebServiceError: + log.debug(u'MusicBrainz is unreachable.') + raise MusicBrainzAPIError('MusicBrainz not reachable', + 'release search', criteria, + traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'release search', criteria, traceback.format_exc()) @@ -351,6 +356,11 @@ def match_track(artist, title): try: res = musicbrainzngs.search_recordings( limit=config['musicbrainz']['searchlimit'].get(int), **criteria) + except musicbrainzngs.WebServiceError: + log.debug(u'MusicBrainz is unreachable.') + raise MusicBrainzAPIError('MusicBrainz not reachable', + 'recording search', criteria, + traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'recording search', criteria, traceback.format_exc()) @@ -383,6 +393,11 @@ def album_for_id(releaseid): except musicbrainzngs.ResponseError: log.debug(u'Album ID match failed.') return None + except musicbrainzngs.WebServiceError: + log.debug(u'MusicBrainz is unreachable.') + raise MusicBrainzAPIError('MusicBrainz not reachable', + 'get release by ID', albumid, + traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'get release by ID', albumid, traceback.format_exc()) @@ -402,6 +417,11 @@ def track_for_id(releaseid): except musicbrainzngs.ResponseError: log.debug(u'Track ID match failed.') return None + except musicbrainzngs.WebServiceError: + log.debug(u'MusicBrainz is unreachable.') + raise MusicBrainzAPIError('MusicBrainz not reachable', + 'get recording by ID', trackid, + traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'get recording by ID', trackid, traceback.format_exc()) From d3fce35481f09f28561dc2438baa7ec5245b7870 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Mon, 26 Jan 2015 17:24:32 +0100 Subject: [PATCH 11/64] Colors are user configurable - Colors are mapped on to a dictionary using abstract names (e.g., text_success) - Add `colors` option under `ui` to allow users to choose their own color scheme - Move configuration option `color` from top-level to `ui` - Show deprecation warning if top-level `color` configuration is used (but respect it) Fix #1238 --- beets/config_default.yaml | 10 +++++++++- beets/ui/__init__.py | 35 +++++++++++++++++++++++++++-------- beets/ui/commands.py | 30 +++++++++++++++++------------- beetsplug/fetchart.py | 6 ++++-- beetsplug/play.py | 5 +++-- 5 files changed, 60 insertions(+), 26 deletions(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 1d3c4ad7a..c448585d9 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -41,7 +41,6 @@ max_filename_length: 0 plugins: [] pluginpath: [] threaded: yes -color: yes timeout: 5.0 per_disc_numbering: no verbose: no @@ -52,6 +51,15 @@ id3v23: no ui: terminal_width: 80 length_diff_thresh: 10.0 + color: yes + colors: + text_success: green + text_warning: yellow + text_error: red + text_highlight: red + text_highlight_minor: lightgray + action_default: turquoise + action: blue list_format_item: $artist - $album - $title list_format_album: $albumartist - $album diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index c541ba2de..633a2c3e6 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -191,7 +191,9 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, is_default = False # Colorize the letter shortcut. - show_letter = colorize('turquoise' if is_default else 'blue', + show_letter = colorize(COLORS['action_default'] + if is_default + else COLORS['action'], show_letter) # Insert the highlighted letter back into the word. @@ -218,7 +220,7 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, if numrange: if isinstance(default, int): default_name = str(default) - default_name = colorize('turquoise', default_name) + default_name = colorize(COLORS['action_default'], default_name) tmpl = '# selection (default %s)' prompt_parts.append(tmpl % default_name) prompt_part_lengths.append(len(tmpl % str(default))) @@ -357,6 +359,13 @@ LIGHT_COLORS = ["darkgray", "red", "green", "yellow", "blue", "fuchsia", "turquoise", "white"] RESET_COLOR = COLOR_ESCAPE + "39;49;00m" +# Map the color names to the configured colors in a dict +COLOR_NAMES = ['text_success', 'text_warning', 'text_error', 'text_highlight', + 'text_highlight_minor', 'action_default', 'action'] +COLORS = dict(zip(COLOR_NAMES, + map(lambda x: config['ui']['colors'][x].get(str), + COLOR_NAMES))) + def _colorize(color, text): """Returns a string that prints the given text in the given color @@ -376,13 +385,14 @@ def colorize(color, text): """Colorize text if colored output is enabled. (Like _colorize but conditional.) """ - if config['color']: + if config['ui']['color']: return _colorize(color, text) else: return text -def _colordiff(a, b, highlight='red', minor_highlight='lightgray'): +def _colordiff(a, b, highlight=COLORS['text_highlight'], + minor_highlight=COLORS['text_highlight_minor']): """Given two values, return the same pair of strings except with their differences highlighted in the specified color. Strings are highlighted intelligently to show differences; other values are @@ -432,11 +442,11 @@ def _colordiff(a, b, highlight='red', minor_highlight='lightgray'): return u''.join(a_out), u''.join(b_out) -def colordiff(a, b, highlight='red'): +def colordiff(a, b, highlight=COLORS['text_highlight']): """Colorize differences between two values if color is enabled. (Like _colordiff but conditional.) """ - if config['color']: + if config['ui']['color']: return _colordiff(a, b, highlight) else: return unicode(a), unicode(b) @@ -546,7 +556,8 @@ def _field_diff(field, old, new): if isinstance(oldval, basestring): oldstr, newstr = colordiff(oldval, newstr) else: - oldstr, newstr = colorize('red', oldstr), colorize('red', newstr) + oldstr = colorize(COLORS['text_error'], oldstr) + newstr = colorize(COLORS['text_error'], newstr) return u'{0} -> {1}'.format(oldstr, newstr) @@ -582,7 +593,7 @@ def show_model_changes(new, old=None, fields=None, always=False): changes.append(u' {0}: {1}'.format( field, - colorize('red', new.formatted()[field]) + colorize(COLORS['text_highlight'], new.formatted()[field]) )) # Print changes. @@ -865,6 +876,14 @@ def _configure(options): else: log.setLevel(logging.INFO) + # Ensure compatibility with old (top-level) color configuration. + # Deprecation msg to motivate user to switch to config['ui']['color]. + if config['color'].exists(): + log.warning(u'Warning: top-level configuration of `color` ' + u'is deprecated. Configure color use under `ui`. ' + u'See documentation for more info.') + config['ui']['color'].set(config['color'].get(bool)) + config_path = config.user_config_path() if os.path.isfile(config_path): log.debug(u'user configuration: {0}', diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 839263fc5..89eff2d49 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -175,11 +175,11 @@ def dist_string(dist): """ out = '%.1f%%' % ((1 - dist) * 100) if dist <= config['match']['strong_rec_thresh'].as_number(): - out = ui.colorize('green', out) + out = ui.colorize(ui.COLORS['text_success'], out) elif dist <= config['match']['medium_rec_thresh'].as_number(): - out = ui.colorize('yellow', out) + out = ui.colorize(ui.COLORS['text_warning'], out) else: - out = ui.colorize('red', out) + out = ui.colorize(ui.COLORS['text_error'], out) return out @@ -196,7 +196,8 @@ def penalty_string(distance, limit=None): if penalties: if limit and len(penalties) > limit: penalties = penalties[:limit] + ['...'] - return ui.colorize('yellow', '(%s)' % ', '.join(penalties)) + return ui.colorize(ui.COLORS['text_warning'], + '(%s)' % ', '.join(penalties)) def show_change(cur_artist, cur_album, match): @@ -269,7 +270,8 @@ def show_change(cur_artist, cur_album, match): # Disambiguation. disambig = disambig_string(match.info) if disambig: - info.append(ui.colorize('lightgray', '(%s)' % disambig)) + info.append(ui.colorize(ui.COLORS['text_highlight_minor'], + '(%s)' % disambig)) print_(' '.join(info)) # Tracks. @@ -314,9 +316,9 @@ def show_change(cur_artist, cur_album, match): cur_track, new_track = format_index(item), format_index(track_info) if cur_track != new_track: if item.track in (track_info.index, track_info.medium_index): - color = 'lightgray' + color = ui.COLORS['text_highlight_minor'] else: - color = 'red' + color = ui.COLORS['text_highlight'] templ = ui.colorize(color, u' (#{0})') lhs += templ.format(cur_track) rhs += templ.format(new_track) @@ -328,7 +330,7 @@ def show_change(cur_artist, cur_album, match): config['ui']['length_diff_thresh'].as_number(): cur_length = ui.human_seconds_short(item.length) new_length = ui.human_seconds_short(track_info.length) - templ = ui.colorize('red', u' ({0})') + templ = ui.colorize(ui.COLORS['text_highlight'], u' ({0})') lhs += templ.format(cur_length) rhs += templ.format(new_length) lhs_width += len(cur_length) + 3 @@ -363,14 +365,14 @@ def show_change(cur_artist, cur_album, match): line = ' ! %s (#%s)' % (track_info.title, format_index(track_info)) if track_info.length: line += ' (%s)' % ui.human_seconds_short(track_info.length) - print_(ui.colorize('yellow', line)) + print_(ui.colorize(ui.COLORS['text_warning'], line)) if match.extra_items: print_('Unmatched tracks:') for item in match.extra_items: line = ' ! %s (#%s)' % (item.title, format_index(item)) if item.length: line += ' (%s)' % ui.human_seconds_short(item.length) - print_(ui.colorize('yellow', line)) + print_(ui.colorize(ui.COLORS['text_warning'], line)) def show_item_change(item, match): @@ -407,7 +409,8 @@ def show_item_change(item, match): # Disambiguation. disambig = disambig_string(match.info) if disambig: - info.append(ui.colorize('lightgray', '(%s)' % disambig)) + info.append(ui.colorize(ui.COLORS['text_highlight_minor'], + '(%s)' % disambig)) print_(' '.join(info)) @@ -566,7 +569,8 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Disambiguation disambig = disambig_string(match.info) if disambig: - line.append(ui.colorize('lightgray', '(%s)' % disambig)) + line.append(ui.colorize(ui.COLORS['text_highlight_minor'], + '(%s)' % disambig)) print_(' '.join(line)) @@ -1000,7 +1004,7 @@ def update_items(lib, query, album, move, pretend): # Item deleted? if not os.path.exists(syspath(item.path)): ui.print_obj(item, lib) - ui.print_(ui.colorize('red', u' deleted')) + ui.print_(ui.colorize(ui.COLORS['text_error'], u' deleted')) if not pretend: item.remove(True) affected_albums.add(item.album_id) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 196be4b2d..ce3e3360d 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -444,9 +444,11 @@ class FetchArtPlugin(plugins.BeetsPlugin): if path: album.set_art(path, False) album.store() - message = ui.colorize('green', 'found album art') + message = ui.colorize(ui.COLORS['text_success'], + 'found album art') else: - message = ui.colorize('red', 'no art found') + message = ui.colorize(ui.COLORS['text_error'], + 'no art found') self._log.info(u'{0.albumartist} - {0.album}: {1}', album, message) diff --git a/beetsplug/play.py b/beetsplug/play.py index 5072234ca..7d0c0a531 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -73,13 +73,14 @@ def play_music(lib, opts, args, log): item_type += 's' if len(selection) > 1 else '' if not selection: - ui.print_(ui.colorize('yellow', 'No {0} to play.'.format(item_type))) + ui.print_(ui.colorize(ui.COLORS['text_warning'], + 'No {0} to play.'.format(item_type))) return # Warn user before playing any huge playlists. if len(selection) > 100: ui.print_(ui.colorize( - 'yellow', + ui.COLORS['text_warning'], 'You are about to queue {0} {1}.'.format(len(selection), item_type) )) From e7f3987b424907b72a00a84e814b00d09212796c Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Mon, 26 Jan 2015 22:19:02 +0100 Subject: [PATCH 12/64] Centralize check and string literal into MusicBrainzAPIError.__init__ for #1190 --- beets/autotag/mb.py | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index 50d146dbb..78baac054 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -38,6 +38,8 @@ class MusicBrainzAPIError(util.HumanReadableException): """ def __init__(self, reason, verb, query, tb=None): self.query = query + if isinstance(reason, musicbrainzngs.WebServiceError): + reason = 'MusicBrainz not reachable' super(MusicBrainzAPIError, self).__init__(reason, verb, tb) def get_message(self): @@ -325,11 +327,6 @@ def match_album(artist, album, tracks=None): try: res = musicbrainzngs.search_releases( limit=config['musicbrainz']['searchlimit'].get(int), **criteria) - except musicbrainzngs.WebServiceError: - log.debug(u'MusicBrainz is unreachable.') - raise MusicBrainzAPIError('MusicBrainz not reachable', - 'release search', criteria, - traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'release search', criteria, traceback.format_exc()) @@ -356,11 +353,6 @@ def match_track(artist, title): try: res = musicbrainzngs.search_recordings( limit=config['musicbrainz']['searchlimit'].get(int), **criteria) - except musicbrainzngs.WebServiceError: - log.debug(u'MusicBrainz is unreachable.') - raise MusicBrainzAPIError('MusicBrainz not reachable', - 'recording search', criteria, - traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'recording search', criteria, traceback.format_exc()) @@ -393,11 +385,6 @@ def album_for_id(releaseid): except musicbrainzngs.ResponseError: log.debug(u'Album ID match failed.') return None - except musicbrainzngs.WebServiceError: - log.debug(u'MusicBrainz is unreachable.') - raise MusicBrainzAPIError('MusicBrainz not reachable', - 'get release by ID', albumid, - traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'get release by ID', albumid, traceback.format_exc()) @@ -417,11 +404,6 @@ def track_for_id(releaseid): except musicbrainzngs.ResponseError: log.debug(u'Track ID match failed.') return None - except musicbrainzngs.WebServiceError: - log.debug(u'MusicBrainz is unreachable.') - raise MusicBrainzAPIError('MusicBrainz not reachable', - 'get recording by ID', trackid, - traceback.format_exc()) except musicbrainzngs.MusicBrainzError as exc: raise MusicBrainzAPIError(exc, 'get recording by ID', trackid, traceback.format_exc()) From 07cea164929ab9d8dd14b3ad4db92e8460007f39 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 27 Jan 2015 19:41:25 +0100 Subject: [PATCH 13/64] Changed the interface of extractart to make it easier to understand what it does. --- beetsplug/embedart.py | 35 ++++++++++++++++++++++------------- docs/changelog.rst | 5 +++-- docs/plugins/embedart.rst | 8 ++++---- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 8e6dbab98..263980b3f 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -88,22 +88,30 @@ class EmbedCoverArtPlugin(BeetsPlugin): help='extract an image from file metadata') extract_cmd.parser.add_option('-o', dest='outpath', help='image output file') - extract_cmd.parser.add_option('-a', dest='albums', action='store_true', - help='extract the art of all matching ' - 'albums') + extract_cmd.parser.add_option('-n', dest='filename', + help='image filename to create for all ' + 'matched albums') + extract_cmd.parser.add_option('-a', dest='associate', + action='store_true', + help='associate the extracted images ' + 'with the album') def extract_func(lib, opts, args): - outpath = opts.outpath or config['art_filename'].get() - if opts.albums: - if opts.outpath and '/' in opts.outpath.replace('\\', '/'): - self._log.error(u"When using -a, only specify a filename " - u"instead of a full path for -o") + if opts.filename: + filename = opts.filename + if os.path.dirname(filename) != '': + self._log.error(u"Only specify a name rather a path for " + u"-n") return for album in lib.albums(decargs(args)): - artpath = normpath(os.path.join(album.path, outpath)) - self.extract_first(artpath, album.items()) + artpath = normpath(os.path.join(album.path, filename)) + artpath = self.extract_first(artpath, album.items()) + if artpath and opts.associate: + album.set_art(artpath) + else: - outpath = normpath(outpath) + outpath = normpath(opts.outpath + or config['art_filename'].get()) self.extract_first(outpath, lib.items(decargs(args))) extract_cmd.func = extract_func @@ -270,8 +278,9 @@ class EmbedCoverArtPlugin(BeetsPlugin): def extract_first(self, outpath, items): for item in items: - if self.extract(outpath, item): - return outpath + real_path = self.extract(outpath, item) + if real_path: + return real_path # 'clearart' command. def clear(self, lib, query): diff --git a/docs/changelog.rst b/docs/changelog.rst index a8dd71823..e61240165 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,8 +26,9 @@ Features: by default. :bug:`1246` * :doc:`/plugins/fetchart`: Names of extracted image art is taken from the ``art_filename`` configuration option. :bug:`1258` -* :doc:`/plugins/fetchart`: New option ``-a`` to extract the cover art of all - matched albums into its directory. :bug:`1261` +* :doc:`/plugins/fetchart`: New option ``-n`` to extract the cover art of all + matched albums into its directory. It's also possible to automatically + associate them with the album when adding ``-a`` :bug:`1261` * :doc:`/plugins/fetchart`: There's a new Wikipedia image source that uses DBpedia to find albums. Thanks to Tom Jaspers. :bug:`1194` diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index 2c8748d14..e556ad90c 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -84,11 +84,11 @@ embedded album art: ``art_filename`` configuration option. It defaults to ``cover`` if it's not specified via ``-o`` nor the config. -* ``beet extractart -a [-o FILE] QUERY``: extracts the images for all albums +* ``beet extractart [-a] [-n FILE] QUERY``: extracts the images for all albums matching the query. The images are placed inside the album folder. The - destination filename is taken either from the ``-o`` option or the - ``art_filename`` configuration option. It defaults to ``cover`` if it's not - specified. + destination filename is taken either from the ``-n`` option. Using ``-a``, + the extracted image files are automatically associated with the corresponding + album. * ``beet clearart QUERY``: removes all embedded images from all items matching the query. (Use with caution!) From 2c75d0567ffafe53564dc4b017a2dcaeb93530fc Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 27 Jan 2015 19:59:49 +0100 Subject: [PATCH 14/64] Made the new functionality the default behaviour. --- beetsplug/embedart.py | 13 ++++++------- docs/changelog.rst | 4 ++-- docs/plugins/embedart.rst | 23 ++++++++++++----------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 263980b3f..10d3b945d 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -97,8 +97,11 @@ class EmbedCoverArtPlugin(BeetsPlugin): 'with the album') def extract_func(lib, opts, args): - if opts.filename: - filename = opts.filename + if opts.outpath: + self.extract_first(normpath(opts.outpath), + lib.items(decargs(args))) + else: + filename = opts.filename or config['art_filename'].get() if os.path.dirname(filename) != '': self._log.error(u"Only specify a name rather a path for " u"-n") @@ -108,11 +111,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): artpath = self.extract_first(artpath, album.items()) if artpath and opts.associate: album.set_art(artpath) - - else: - outpath = normpath(opts.outpath - or config['art_filename'].get()) - self.extract_first(outpath, lib.items(decargs(args))) + album.store() extract_cmd.func = extract_func # Clear command. diff --git a/docs/changelog.rst b/docs/changelog.rst index e61240165..7041bba13 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,11 +24,11 @@ Features: * :doc:`plugins/mbsync`: A new ``-f/--format`` option controls the output format when listing unrecognized items. The output is also now more helpful by default. :bug:`1246` -* :doc:`/plugins/fetchart`: Names of extracted image art is taken from the - ``art_filename`` configuration option. :bug:`1258` * :doc:`/plugins/fetchart`: New option ``-n`` to extract the cover art of all matched albums into its directory. It's also possible to automatically associate them with the album when adding ``-a`` :bug:`1261` +* :doc:`/plugins/fetchart`: Names of extracted image art is taken from the + ``art_filename`` configuration option. :bug:`1258` * :doc:`/plugins/fetchart`: There's a new Wikipedia image source that uses DBpedia to find albums. Thanks to Tom Jaspers. :bug:`1194` diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index e556ad90c..cf96c504f 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -77,18 +77,19 @@ embedded album art: use a specific image file from the filesystem; otherwise, each album embeds its own currently associated album art. -* ``beet extractart [-o FILE] QUERY``: extracts the image from an item matching - the query and stores it in a file. You can specify the destination file using - the ``-o`` option, but leave off the extension: it will be chosen - automatically. The destination filename is specified using the - ``art_filename`` configuration option. It defaults to ``cover`` if it's not - specified via ``-o`` nor the config. - * ``beet extractart [-a] [-n FILE] QUERY``: extracts the images for all albums - matching the query. The images are placed inside the album folder. The - destination filename is taken either from the ``-n`` option. Using ``-a``, - the extracted image files are automatically associated with the corresponding - album. + matching the query. The images are placed inside the album folder. You can + specify the destination file name using the ``-n`` option, but leave off the + extension: it will be chosen automatically. The destination filename is + specified using the ``art_filename`` configuration option. It defaults to + ``cover`` if it's not specified via ``-o`` nor the config. + Using ``-a``, the extracted image files are automatically associated with the + corresponding album. + +* ``beet extractart -o FILE QUERY``: extracts the image from an item matching + the query and stores it in a file. You have to specify the destination file + using the ``-o`` option, but leave off the extension: it will be chosen + automatically. * ``beet clearart QUERY``: removes all embedded images from all items matching the query. (Use with caution!) From ac3ea16656d33e712616e491c0639f1d2053b3ad Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 27 Jan 2015 13:48:13 -0800 Subject: [PATCH 15/64] Fix for unicode_literals in scrub Here's an example where unicode_literals may not be a great idea: these should probably be "native" strings, i.e., bytes on 2.x and unicode on 3.x. --- beetsplug/scrub.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index a832c9466..5c0d74e6e 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -26,21 +26,21 @@ from beets import config from beets import mediafile _MUTAGEN_FORMATS = { - 'asf': 'ASF', - 'apev2': 'APEv2File', - 'flac': 'FLAC', - 'id3': 'ID3FileType', - 'mp3': 'MP3', - 'mp4': 'MP4', - 'oggflac': 'OggFLAC', - 'oggspeex': 'OggSpeex', - 'oggtheora': 'OggTheora', - 'oggvorbis': 'OggVorbis', - 'oggopus': 'OggOpus', - 'trueaudio': 'TrueAudio', - 'wavpack': 'WavPack', - 'monkeysaudio': 'MonkeysAudio', - 'optimfrog': 'OptimFROG', + b'asf': b'ASF', + b'apev2': b'APEv2File', + b'flac': b'FLAC', + b'id3': b'ID3FileType', + b'mp3': b'MP3', + b'mp4': b'MP4', + b'oggflac': b'OggFLAC', + b'oggspeex': b'OggSpeex', + b'oggtheora': b'OggTheora', + b'oggvorbis': b'OggVorbis', + b'oggopus': b'OggOpus', + b'trueaudio': b'TrueAudio', + b'wavpack': b'WavPack', + b'monkeysaudio': b'MonkeysAudio', + b'optimfrog': b'OptimFROG', } @@ -103,7 +103,7 @@ class ScrubPlugin(BeetsPlugin): """ classes = [] for modname, clsname in _MUTAGEN_FORMATS.items(): - mod = __import__('mutagen.{0}'.format(modname), + mod = __import__(b'mutagen.{0}'.format(modname), fromlist=[clsname]) classes.append(getattr(mod, clsname)) return classes From 84b42b6f1e09f1a47618442da73970fd2cce0ef6 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 27 Jan 2015 14:04:12 -0800 Subject: [PATCH 16/64] Tox: Do not conflate 2.6 with `setup.py test` It was confusing to have different ways of running the tests for the two versions. This also reduced the verbosity, making Travis' web interface more useful. --- tox.ini | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index c1556533a..a4c2be05d 100644 --- a/tox.ini +++ b/tox.ini @@ -17,14 +17,12 @@ deps = rarfile responses commands = - nosetests -v {posargs} + nosetests {posargs} [testenv:py26] deps = {[testenv]deps} unittest2 -commands = - python ./setup.py test {posargs} [testenv:py27cov] basepython = python2.7 @@ -32,7 +30,12 @@ deps = {[testenv]deps} coverage commands = - nosetests -v --with-coverage {posargs} + nosetests --with-coverage {posargs} + +[testenv:py27setup] +basepython = python2.7 +commands = + python ./setup.py test {posargs} [testenv:docs] changedir = docs From 790c41a73d0aacff94a73266600582fe03b0daba Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 27 Jan 2015 15:03:19 -0800 Subject: [PATCH 17/64] write: Do not try to write non-writable fields Fix #1268. --- beets/library.py | 8 ++++++++ beets/ui/commands.py | 2 +- docs/changelog.rst | 2 ++ test/test_ui.py | 31 +++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index 33aa314bf..15151e9fe 100644 --- a/beets/library.py +++ b/beets/library.py @@ -402,6 +402,14 @@ class Item(LibModel): `write`. """ + _media_tag_fields = set(MediaFile.fields()).intersection(_fields.keys()) + """Set of item fields that are backed by *writable* `MediaFile` tag + fields. + + This excludes fields that represent audio data, such as `bitrate` or + `length`. + """ + _formatter = FormattedItemMapping _sorts = {'artist': SmartArtistSort} diff --git a/beets/ui/commands.py b/beets/ui/commands.py index f6c1312d2..7446fb0b5 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1428,7 +1428,7 @@ def write_items(lib, query, pretend, force): # Check for and display changes. changed = ui.show_model_changes(item, clean_item, - library.Item._media_fields, force) + library.Item._media_tag_fields, force) if (changed or force) and not pretend: item.try_sync() diff --git a/docs/changelog.rst b/docs/changelog.rst index b18b05081..6b3893ce9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -75,6 +75,8 @@ Fixes: * :doc:`/plugins/importfeeds` and :doc:`/plugins/smartplaylist`: Automatically create parent directories for playlist files (instead of crashing when the parent directory does not exist). :bug:`1266` +* The :ref:`write-cmd` command no longer tries to "write" non-writable fields + like the bitrate. :bug:`1268` For developers: diff --git a/test/test_ui.py b/test/test_ui.py index 6b40d6cca..1fbb8d210 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -318,6 +318,37 @@ class WriteTest(unittest.TestCase, TestHelper): item = self.lib.items().get() self.assertEqual(item.mtime, item.current_mtime()) + def test_non_metadata_field_unchanged(self): + """Changing a non-"tag" field like `bitrate` and writing should + have no effect. + """ + # An item that starts out "clean". + item = self.add_item_fixture() + item.read() + + # ... but with a mismatched bitrate. + item.bitrate = 123 + item.store() + + with capture_stdout() as stdout: + self.write_cmd() + + self.assertEqual(stdout.getvalue(), '') + + def test_write_metadata_field(self): + item = self.add_item_fixture() + item.read() + old_title = item.title + + item.title = 'new title' + item.store() + + with capture_stdout() as stdout: + self.write_cmd() + + self.assertTrue('{0} -> new title'.format(old_title) + in stdout.getvalue()) + class MoveTest(_common.TestCase): def setUp(self): From 61c7c837ec01ad46e5d28044a8818473bbdf721e Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 28 Jan 2015 11:13:54 +0100 Subject: [PATCH 18/64] Fix byte string management in ui.print_() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Joining byte strings → prefix ' ' with b --- beets/ui/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 558030a25..687d4b320 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -107,7 +107,7 @@ def print_(*strings): if isinstance(strings[0], unicode): txt = u' '.join(strings) else: - txt = ' '.join(strings) + txt = b' '.join(strings) else: txt = u'' if isinstance(txt, unicode): From 67ecf32671965dfbc945774f80d61afb9f711a14 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Wed, 28 Jan 2015 15:46:16 +0100 Subject: [PATCH 19/64] Fixed typos. --- beetsplug/embedart.py | 4 ++-- docs/changelog.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 10d3b945d..6e3783030 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -103,8 +103,8 @@ class EmbedCoverArtPlugin(BeetsPlugin): else: filename = opts.filename or config['art_filename'].get() if os.path.dirname(filename) != '': - self._log.error(u"Only specify a name rather a path for " - u"-n") + self._log.error(u"Only specify a name rather than a path " + u"for -n") return for album in lib.albums(decargs(args)): artpath = normpath(os.path.join(album.path, filename)) diff --git a/docs/changelog.rst b/docs/changelog.rst index efe2dc11f..30d26525c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,7 +26,7 @@ Features: by default. :bug:`1246` * :doc:`/plugins/fetchart`: New option ``-n`` to extract the cover art of all matched albums into its directory. It's also possible to automatically - associate them with the album when adding ``-a`` :bug:`1261` + associate them with the album when adding ``-a``. :bug:`1261` * :doc:`/plugins/fetchart`: Names of extracted image art is taken from the ``art_filename`` configuration option. :bug:`1258` * :doc:`/plugins/fetchart`: There's a new Wikipedia image source that uses From fc82f2bb894e368fbf31ac9baf9a4bfe4d16530e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 28 Jan 2015 11:43:35 -0800 Subject: [PATCH 20/64] Changelog for #1190/#1272 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 30d26525c..bff60c274 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -80,6 +80,8 @@ Fixes: parent directory does not exist). :bug:`1266` * The :ref:`write-cmd` command no longer tries to "write" non-writable fields like the bitrate. :bug:`1268` +* The error message when MusicBrainz is not reachable on the network is now + much clearer. Thanks to Tom Jaspers. :bug:`1190` :bug:`1272` For developers: From e7378c77a73b5a7ef27b9d153004f01feb490343 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Thu, 29 Jan 2015 14:04:37 +0100 Subject: [PATCH 21/64] Fix tests to use config['ui']['color'] instead of top-level color --- test/helper.py | 2 +- test/test_ui.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/helper.py b/test/helper.py index a1faf2a63..2f0889bc0 100644 --- a/test/helper.py +++ b/test/helper.py @@ -165,7 +165,7 @@ class TestHelper(object): self.config['plugins'] = [] self.config['verbose'] = True - self.config['color'] = False + self.config['ui']['color'] = False self.config['threaded'] = False self.libdir = os.path.join(self.temp_dir, 'libdir') diff --git a/test/test_ui.py b/test/test_ui.py index e32f9ed83..05e3b53c2 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -888,7 +888,7 @@ class ShowChangeTest(_common.TestCase): items = items or self.items info = info or self.info mapping = dict(zip(items, info.tracks)) - config['color'] = False + config['ui']['color'] = False album_dist = distance(items, info, mapping) album_dist._penalties = {'album': [dist]} commands.show_change( From ea687baebdd173490af60d5a7e7d65ff591ee257 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Thu, 29 Jan 2015 14:05:00 +0100 Subject: [PATCH 22/64] Configurable colors: update documentation and changelog --- docs/changelog.rst | 4 ++++ docs/reference/config.rst | 50 +++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c10a64981..0cfc279ce 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,10 @@ Changelog Features: +* The colors used are now configurable via the new config option ``colors``, + nested under the option ``ui``. The `color` config option has been moved + from top-level to under ``ui``. Beets will respect the old color setting, + but will warn the user with a deprecation message. :bug:`1238` * A new :doc:`/plugins/filefilter` lets you write regular expressions to automatically avoid importing certain files. Thanks to :user:`mried`. :bug:`1186` diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 7782e4ad8..a64e73749 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -162,13 +162,6 @@ Either ``yes`` or ``no``, indicating whether the autotagger should use multiple threads. This makes things faster but may behave strangely. Defaults to ``yes``. -color -~~~~~ - -Either ``yes`` or ``no``; whether to use color in console output (currently -only in the ``import`` command). Turn this off if your terminal doesn't -support ANSI colors. - .. _list_format_item: list_format_item @@ -277,6 +270,49 @@ version of ID3. Enable this option to instead use the older ID3v2.3 standard, which is preferred by certain older software such as Windows Media Player. +UI Options +---------- + +The options that allow for customization of the visual appearance +of the console interface. + +These options are available in this section: + +color +~~~~~ + +Either ``yes`` or ``no``; whether to use color in console output (currently +only in the ``import`` command). Turn this off if your terminal doesn't +support ANSI colors. + +.. note:: + + The `color` option was previously a top-level configuration. This is + still respected, but a deprecation message will be shown until your + top-level `color` configuration has been nested under `ui`. + +colors +~~~~~~ + +The colors that are used throughout the user interface. These are only used if +the ``color`` option is set to ``yes``. For example, you might have a section +in your configuration file that looks like this:: + + ui: + color: yes + colors: + text_success: green + text_warning: yellow + text_error: red + text_highlight: red + text_highlight_minor: lightgray + action_default: turquoise + action: blue + +Available colors: black, darkred, darkgreen, brown, darkblue, purple, teal, +lightgray, darkgray, red, green, yellow, blue, fuchsia, turquoise, white + + Importer Options ---------------- From f483012183b1f56456c2c7005a5fbf5244f63b5e Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Thu, 29 Jan 2015 14:33:57 +0100 Subject: [PATCH 23/64] Colorize is now to be called with the abstract color_name instead of the color. E.g., `colorize('text_success', 'hello world')` To ensure compatibility with 3rd party plugins, a valid color ('red') can still be passed, but it will be logged. --- beets/ui/__init__.py | 26 +++++++++++++++----------- beets/ui/commands.py | 29 +++++++++++++---------------- beetsplug/fetchart.py | 6 ++---- beetsplug/play.py | 4 ++-- 4 files changed, 32 insertions(+), 33 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 633a2c3e6..9efd0c706 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -191,9 +191,7 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, is_default = False # Colorize the letter shortcut. - show_letter = colorize(COLORS['action_default'] - if is_default - else COLORS['action'], + show_letter = colorize('action_default' if is_default else 'action', show_letter) # Insert the highlighted letter back into the word. @@ -220,7 +218,7 @@ def input_options(options, require=False, prompt=None, fallback_prompt=None, if numrange: if isinstance(default, int): default_name = str(default) - default_name = colorize(COLORS['action_default'], default_name) + default_name = colorize('action_default', default_name) tmpl = '# selection (default %s)' prompt_parts.append(tmpl % default_name) prompt_part_lengths.append(len(tmpl % str(default))) @@ -381,18 +379,24 @@ def _colorize(color, text): return escape + text + RESET_COLOR -def colorize(color, text): +def colorize(color_name, text): """Colorize text if colored output is enabled. (Like _colorize but conditional.) """ if config['ui']['color']: + # In case a 3rd party plugin is still passing the actual color ('red') + # instead of the abstract color name ('text_error') + color = COLORS.get(color_name) + if not color: + log.debug(u'Invalid color_name: {0}', color_name) + color = color_name return _colorize(color, text) else: return text -def _colordiff(a, b, highlight=COLORS['text_highlight'], - minor_highlight=COLORS['text_highlight_minor']): +def _colordiff(a, b, highlight='text_highlight', + minor_highlight='text_highlight_minor'): """Given two values, return the same pair of strings except with their differences highlighted in the specified color. Strings are highlighted intelligently to show differences; other values are @@ -442,7 +446,7 @@ def _colordiff(a, b, highlight=COLORS['text_highlight'], return u''.join(a_out), u''.join(b_out) -def colordiff(a, b, highlight=COLORS['text_highlight']): +def colordiff(a, b, highlight='text_highlight'): """Colorize differences between two values if color is enabled. (Like _colordiff but conditional.) """ @@ -556,8 +560,8 @@ def _field_diff(field, old, new): if isinstance(oldval, basestring): oldstr, newstr = colordiff(oldval, newstr) else: - oldstr = colorize(COLORS['text_error'], oldstr) - newstr = colorize(COLORS['text_error'], newstr) + oldstr = colorize('text_error', oldstr) + newstr = colorize('text_error', newstr) return u'{0} -> {1}'.format(oldstr, newstr) @@ -593,7 +597,7 @@ def show_model_changes(new, old=None, fields=None, always=False): changes.append(u' {0}: {1}'.format( field, - colorize(COLORS['text_highlight'], new.formatted()[field]) + colorize('text_highlight', new.formatted()[field]) )) # Print changes. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 89eff2d49..710e7bd4d 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -175,11 +175,11 @@ def dist_string(dist): """ out = '%.1f%%' % ((1 - dist) * 100) if dist <= config['match']['strong_rec_thresh'].as_number(): - out = ui.colorize(ui.COLORS['text_success'], out) + out = ui.colorize('text_success', out) elif dist <= config['match']['medium_rec_thresh'].as_number(): - out = ui.colorize(ui.COLORS['text_warning'], out) + out = ui.colorize('text_warning', out) else: - out = ui.colorize(ui.COLORS['text_error'], out) + out = ui.colorize('text_error', out) return out @@ -196,8 +196,7 @@ def penalty_string(distance, limit=None): if penalties: if limit and len(penalties) > limit: penalties = penalties[:limit] + ['...'] - return ui.colorize(ui.COLORS['text_warning'], - '(%s)' % ', '.join(penalties)) + return ui.colorize('text_warning', '(%s)' % ', '.join(penalties)) def show_change(cur_artist, cur_album, match): @@ -270,8 +269,7 @@ def show_change(cur_artist, cur_album, match): # Disambiguation. disambig = disambig_string(match.info) if disambig: - info.append(ui.colorize(ui.COLORS['text_highlight_minor'], - '(%s)' % disambig)) + info.append(ui.colorize('text_highlight_minor', '(%s)' % disambig)) print_(' '.join(info)) # Tracks. @@ -316,9 +314,9 @@ def show_change(cur_artist, cur_album, match): cur_track, new_track = format_index(item), format_index(track_info) if cur_track != new_track: if item.track in (track_info.index, track_info.medium_index): - color = ui.COLORS['text_highlight_minor'] + color = 'text_highlight_minor' else: - color = ui.COLORS['text_highlight'] + color = 'text_highlight' templ = ui.colorize(color, u' (#{0})') lhs += templ.format(cur_track) rhs += templ.format(new_track) @@ -330,7 +328,7 @@ def show_change(cur_artist, cur_album, match): config['ui']['length_diff_thresh'].as_number(): cur_length = ui.human_seconds_short(item.length) new_length = ui.human_seconds_short(track_info.length) - templ = ui.colorize(ui.COLORS['text_highlight'], u' ({0})') + templ = ui.colorize('text_highlight', u' ({0})') lhs += templ.format(cur_length) rhs += templ.format(new_length) lhs_width += len(cur_length) + 3 @@ -365,14 +363,14 @@ def show_change(cur_artist, cur_album, match): line = ' ! %s (#%s)' % (track_info.title, format_index(track_info)) if track_info.length: line += ' (%s)' % ui.human_seconds_short(track_info.length) - print_(ui.colorize(ui.COLORS['text_warning'], line)) + print_(ui.colorize('text_warning', line)) if match.extra_items: print_('Unmatched tracks:') for item in match.extra_items: line = ' ! %s (#%s)' % (item.title, format_index(item)) if item.length: line += ' (%s)' % ui.human_seconds_short(item.length) - print_(ui.colorize(ui.COLORS['text_warning'], line)) + print_(ui.colorize('text_warning', line)) def show_item_change(item, match): @@ -409,8 +407,7 @@ def show_item_change(item, match): # Disambiguation. disambig = disambig_string(match.info) if disambig: - info.append(ui.colorize(ui.COLORS['text_highlight_minor'], - '(%s)' % disambig)) + info.append(ui.colorize('text_highlight_minor', '(%s)' % disambig)) print_(' '.join(info)) @@ -569,7 +566,7 @@ def choose_candidate(candidates, singleton, rec, cur_artist=None, # Disambiguation disambig = disambig_string(match.info) if disambig: - line.append(ui.colorize(ui.COLORS['text_highlight_minor'], + line.append(ui.colorize('text_highlight_minor', '(%s)' % disambig)) print_(' '.join(line)) @@ -1004,7 +1001,7 @@ def update_items(lib, query, album, move, pretend): # Item deleted? if not os.path.exists(syspath(item.path)): ui.print_obj(item, lib) - ui.print_(ui.colorize(ui.COLORS['text_error'], u' deleted')) + ui.print_(ui.colorize('text_error', u' deleted')) if not pretend: item.remove(True) affected_albums.add(item.album_id) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index ce3e3360d..5af21d23a 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -444,11 +444,9 @@ class FetchArtPlugin(plugins.BeetsPlugin): if path: album.set_art(path, False) album.store() - message = ui.colorize(ui.COLORS['text_success'], - 'found album art') + message = ui.colorize('text_success', 'found album art') else: - message = ui.colorize(ui.COLORS['text_error'], - 'no art found') + message = ui.colorize('text_error', 'no art found') self._log.info(u'{0.albumartist} - {0.album}: {1}', album, message) diff --git a/beetsplug/play.py b/beetsplug/play.py index 7d0c0a531..6707c8411 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -73,14 +73,14 @@ def play_music(lib, opts, args, log): item_type += 's' if len(selection) > 1 else '' if not selection: - ui.print_(ui.colorize(ui.COLORS['text_warning'], + ui.print_(ui.colorize('text_warning', 'No {0} to play.'.format(item_type))) return # Warn user before playing any huge playlists. if len(selection) > 100: ui.print_(ui.colorize( - ui.COLORS['text_warning'], + 'text_warning', 'You are about to queue {0} {1}.'.format(len(selection), item_type) )) From 0947b8f286764e5a13c4ce0ce504dd8e7e38ea8a Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Fri, 30 Jan 2015 12:13:07 +0100 Subject: [PATCH 24/64] Move color-lookup from config in to the `colorize` function The mapping occurs lazily (and only once); now in a more pythonic style --- beets/ui/__init__.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 9efd0c706..9b71de0ae 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -357,12 +357,11 @@ LIGHT_COLORS = ["darkgray", "red", "green", "yellow", "blue", "fuchsia", "turquoise", "white"] RESET_COLOR = COLOR_ESCAPE + "39;49;00m" -# Map the color names to the configured colors in a dict +# These abstract COLOR_NAMES are lazily mapped on to the actual color in COLORS +# as they are defined in the configuration files, see function: colorize COLOR_NAMES = ['text_success', 'text_warning', 'text_error', 'text_highlight', 'text_highlight_minor', 'action_default', 'action'] -COLORS = dict(zip(COLOR_NAMES, - map(lambda x: config['ui']['colors'][x].get(str), - COLOR_NAMES))) +COLORS = None def _colorize(color, text): @@ -384,6 +383,10 @@ def colorize(color_name, text): conditional.) """ if config['ui']['color']: + global COLORS + if not COLORS: + COLORS = dict((name, config['ui']['colors'][name].get(unicode)) + for name in COLOR_NAMES) # In case a 3rd party plugin is still passing the actual color ('red') # instead of the abstract color name ('text_error') color = COLORS.get(color_name) From a608a5fc36d27e9cbc5fda5dad4e247532a6af37 Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sat, 31 Jan 2015 12:41:01 +0100 Subject: [PATCH 25/64] Plugins are able to return a list of import tasks to create instead of the original import task using the import_task_created event. Needed for #1167 --- beets/importer.py | 58 +++++++++++++++++++++++++++++--------------- test/test_plugins.py | 5 ++-- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index f7d6aedba..973097f9e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -547,9 +547,22 @@ class ImportTask(object): plugins.send('album_imported', lib=lib, album=self.album) def emit_created(self, session): - """Send the `import_task_created` event for this task. + """Send the `import_task_created` event for this task and return a list + of tasks which might be returned by plugins. """ - plugins.send('import_task_created', session=session, task=self) + tasks = plugins.send('import_task_created', session=session, task=self) + if not tasks: + tasks = [self] + else: + # The plugins gave us a list of lists of task. Flatten it. + flat_tasks = [] + for inner in tasks: + if isinstance(inner, list): + flat_tasks += inner + else: + flat_tasks.append(inner) + tasks = [t for t in flat_tasks if t] + return tasks def lookup_candidates(self): """Retrieve and store candidates for this album. @@ -1006,15 +1019,17 @@ class ImportTaskFactory(object): for dirs, paths in self.paths(): if self.session.config['singletons']: for path in paths: - task = self._create(self.singleton(path)) - if task: - yield task + tasks = self._create(self.singleton(path)) + if tasks: + for task in tasks: + yield task yield self.sentinel(dirs) else: - task = self._create(self.album(paths, dirs)) - if task: - yield task + tasks = self._create(self.album(paths, dirs)) + if tasks: + for task in tasks: + yield task # Produce the final sentinel for this toppath to indicate that # it is finished. This is usually just a SentinelImportTask, but @@ -1033,10 +1048,11 @@ class ImportTaskFactory(object): task. If `task` is None, do nothing. """ if task: - task.emit_created(self.session) - if not task.skip: - self.imported += 1 - return task + tasks = task.emit_created(self.session) + for task in tasks: + if not task.skip: + self.imported += 1 + return tasks def paths(self): """Walk `self.toppath` and yield `(dirs, files)` pairs where @@ -1189,8 +1205,9 @@ def query_tasks(session): # Search for items. for item in session.lib.items(session.query): task = SingletonImportTask(None, item) - task.emit_created(session) - yield task + tasks = task.emit_created(session) + for task in tasks: + yield task else: # Search for albums. @@ -1206,8 +1223,9 @@ def query_tasks(session): item.album_id = None task = ImportTask(None, [album.item_dir()], items) - task.emit_created(session) - yield task + tasks = task.emit_created(session) + for task in tasks: + yield task @pipeline.mutator_stage @@ -1254,8 +1272,9 @@ def user_query(session, task): def emitter(task): for item in task.items: task = SingletonImportTask(task.toppath, item) - task.emit_created(session) - yield task + tasks = task.emit_created(session) + for new_task in tasks: + yield new_task yield SentinelImportTask(task.toppath, task.paths) ipl = pipeline.Pipeline([ @@ -1394,8 +1413,7 @@ def group_albums(session): tasks = [] for _, items in itertools.groupby(task.items, group): task = ImportTask(items=list(items)) - task.emit_created(session) - tasks.append(task) + tasks += task.emit_created(session) tasks.append(SentinelImportTask(task.toppath, task.paths)) task = pipeline.multiple(tasks) diff --git a/test/test_plugins.py b/test/test_plugins.py index 3b8d79cab..056be14e4 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -243,9 +243,8 @@ class EventsTest(unittest.TestCase, ImportHelper, TestHelper): logs = [line for line in logs if not line.startswith('Sending event:')] self.assertEqual(logs, [ - 'Album: {0}/album'.format(self.import_dir), - ' {0}'.format(self.file_paths[0]), - ' {0}'.format(self.file_paths[1]), + 'Singleton: {0}'.format(self.file_paths[0]), + 'Singleton: {0}'.format(self.file_paths[1]), ]) From 557330e994e6f645b42ffa2f31fe9f82ca3970b2 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sat, 31 Jan 2015 14:16:15 +0100 Subject: [PATCH 26/64] Fix open numeric ranges Also improve InvalidQueryError message and update NumericQuery._convert() docstring. Fix #1288. --- beets/dbcore/query.py | 10 +++++++--- test/test_query.py | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 8379b725a..2f90e0398 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -25,7 +25,7 @@ from datetime import datetime, timedelta class InvalidQueryError(ValueError): def __init__(self, what, expected, detail=None): - message = "{0!r} is not {1}".format(what, expected) + message = "'{0}' is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) super(InvalidQueryError, self).__init__(message) @@ -214,10 +214,14 @@ class NumericQuery(FieldQuery): a float. """ def _convert(self, s): - """Convert a string to a numeric type (float or int). If the - string cannot be converted, return None. + """Convert a string to a numeric type (float or int). + + Return None if `s` is empty. + Raise an InvalidQueryError if the string cannot be converted. """ # This is really just a bit of fun premature optimization. + if not s: + return None try: return int(s) except ValueError: diff --git a/test/test_query.py b/test/test_query.py index 065b95623..c76ddf11b 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -334,6 +334,9 @@ class MatchTest(_common.TestCase): q = dbcore.query.NumericQuery('bitrate', '200000..300000') self.assertFalse(q.match(self.item)) + def test_open_range(self): + dbcore.query.NumericQuery('bitrate', '100000..') + class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def setUp(self): From e99adddb1124e5a5636e37feb42e6ecfb2dbe755 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sat, 31 Jan 2015 15:36:39 +0100 Subject: [PATCH 27/64] Importer: byte strings for multi-disc directories Make regexes from raw byte strings and not unicode. Update the tests. Fix #1285 --- beets/importer.py | 5 +++-- test/test_importer.py | 34 +++++++++++++++++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index f7d6aedba..4f73c768e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1457,7 +1457,7 @@ def albums_in_dir(path): match = marker_pat.match(subdir) if match: subdir_pat = re.compile( - r'^%s\d' % re.escape(match.group(1)), re.I + b'^%s\d' % re.escape(match.group(1)), re.I ) else: start_collapsing = False @@ -1478,8 +1478,9 @@ def albums_in_dir(path): start_collapsing = True # Set the current pattern to match directories with the same # prefix as this one, followed by a digit. + print(repr(re.escape(match.group(1)))) collapse_pat = re.compile( - r'^%s\d' % re.escape(match.group(1)), re.I + b'^%s\d' % re.escape(match.group(1)), re.I ) break diff --git a/test/test_importer.py b/test/test_importer.py index 48a489258..74d82a6a1 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1383,38 +1383,38 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): def setUp(self): super(MultiDiscAlbumsInDirTest, self).setUp() - self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir')) + self.base = os.path.abspath(os.path.join(self.temp_dir, b'tempdir')) os.mkdir(self.base) self.dirs = [ # Nested album, multiple subdirs. # Also, false positive marker in root dir, and subtitle for disc 3. - os.path.join(self.base, 'ABCD1234'), - os.path.join(self.base, 'ABCD1234', 'cd 1'), - os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus'), + os.path.join(self.base, b'ABCD1234'), + os.path.join(self.base, b'ABCD1234', b'cd 1'), + os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus'), # Nested album, single subdir. # Also, punctuation between marker and disc number. - os.path.join(self.base, 'album'), - os.path.join(self.base, 'album', 'cd _ 1'), + os.path.join(self.base, b'album'), + os.path.join(self.base, b'album', b'cd _ 1'), # Flattened album, case typo. # Also, false positive marker in parent dir. - os.path.join(self.base, 'artist [CD5]'), - os.path.join(self.base, 'artist [CD5]', 'CAT disc 1'), - os.path.join(self.base, 'artist [CD5]', 'CAt disc 2'), + os.path.join(self.base, b'artist [CD5]'), + os.path.join(self.base, b'artist [CD5]', b'CÀT disc 1'), + os.path.join(self.base, b'artist [CD5]', b'CÀt disc 2'), # Single disc album, sorted between CAT discs. - os.path.join(self.base, 'artist [CD5]', 'CATS'), + os.path.join(self.base, b'artist [CD5]', b'CÀTS'), ] self.files = [ - os.path.join(self.base, 'ABCD1234', 'cd 1', 'song1.mp3'), - os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song2.mp3'), - os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song3.mp3'), - os.path.join(self.base, 'album', 'cd _ 1', 'song4.mp3'), - os.path.join(self.base, 'artist [CD5]', 'CAT disc 1', 'song5.mp3'), - os.path.join(self.base, 'artist [CD5]', 'CAt disc 2', 'song6.mp3'), - os.path.join(self.base, 'artist [CD5]', 'CATS', 'song7.mp3'), + os.path.join(self.base, b'ABCD1234', b'cd 1', b'song1.mp3'), + os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song2.mp3'), + os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song3.mp3'), + os.path.join(self.base, b'album', b'cd _ 1', b'song4.mp3'), + os.path.join(self.base, b'artist [CD5]', b'CÀT disc 1', b'song5.mp3'), + os.path.join(self.base, b'artist [CD5]', b'CÀt disc 2', b'song6.mp3'), + os.path.join(self.base, b'artist [CD5]', b'CÀTS', b'song7.mp3'), ] for path in self.dirs: From f284d8fad58563e98999d9630177a87fe7ea2abc Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sat, 31 Jan 2015 19:47:19 +0100 Subject: [PATCH 28/64] Handle shlex parse errors in query strings Provide context: offending query string. Update changelog. Fix #1290. --- beets/library.py | 5 ++++- docs/changelog.rst | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index 15151e9fe..d521fc4a8 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1069,7 +1069,10 @@ def parse_query_string(s, model_cls): # http://bugs.python.org/issue6988 if isinstance(s, unicode): s = s.encode('utf8') - parts = [p.decode('utf8') for p in shlex.split(s)] + try: + parts = [p.decode('utf8') for p in shlex.split(s)] + except ValueError as exc: + raise ValueError("Cannot parse {0!r} (error was: {1})".format(s, exc)) return parse_query_parts(parts, model_cls) diff --git a/docs/changelog.rst b/docs/changelog.rst index 038e6f93d..b8fc19dac 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -86,6 +86,7 @@ Fixes: like the bitrate. :bug:`1268` * The error message when MusicBrainz is not reachable on the network is now much clearer. Thanks to Tom Jaspers. :bug:`1190` :bug:`1272` +* Improve error messages when parsing query strings with shlex. :bug:`1290` For developers: From 614fbf20ca03b4b2b73fda03039612257f5c714e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 31 Jan 2015 11:24:27 -0800 Subject: [PATCH 29/64] Tests for #1285: parameterize tests Also remove an errant `print` and use `rb''` literals for regexes. --- beets/importer.py | 5 ++--- test/test_importer.py | 40 +++++++++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 4f73c768e..ab7008aed 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1457,7 +1457,7 @@ def albums_in_dir(path): match = marker_pat.match(subdir) if match: subdir_pat = re.compile( - b'^%s\d' % re.escape(match.group(1)), re.I + br'^%s\d' % re.escape(match.group(1)), re.I ) else: start_collapsing = False @@ -1478,9 +1478,8 @@ def albums_in_dir(path): start_collapsing = True # Set the current pattern to match directories with the same # prefix as this one, followed by a digit. - print(repr(re.escape(match.group(1)))) collapse_pat = re.compile( - b'^%s\d' % re.escape(match.group(1)), re.I + br'^%s\d' % re.escape(match.group(1)), re.I ) break diff --git a/test/test_importer.py b/test/test_importer.py index 74d82a6a1..3a36fd65d 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1380,12 +1380,19 @@ class AlbumsInDirTest(_common.TestCase): class MultiDiscAlbumsInDirTest(_common.TestCase): - def setUp(self): - super(MultiDiscAlbumsInDirTest, self).setUp() + def create_music(self, files=True, ascii=True): + """Create some music in multiple album directories. + `files` indicates whether to create the files (otherwise, only + directories are made). `ascii` indicates ACII-only filenames; + otherwise, we use Unicode names. + """ self.base = os.path.abspath(os.path.join(self.temp_dir, b'tempdir')) os.mkdir(self.base) + name = b'CAT' if ascii else b'CÁT' + name_alt_case = b'CAt' if ascii else b'CÁt' + self.dirs = [ # Nested album, multiple subdirs. # Also, false positive marker in root dir, and subtitle for disc 3. @@ -1401,28 +1408,34 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): # Flattened album, case typo. # Also, false positive marker in parent dir. os.path.join(self.base, b'artist [CD5]'), - os.path.join(self.base, b'artist [CD5]', b'CÀT disc 1'), - os.path.join(self.base, b'artist [CD5]', b'CÀt disc 2'), + os.path.join(self.base, b'artist [CD5]', name + b' disc 1'), + os.path.join(self.base, b'artist [CD5]', + name_alt_case + b' disc 2'), # Single disc album, sorted between CAT discs. - os.path.join(self.base, b'artist [CD5]', b'CÀTS'), + os.path.join(self.base, b'artist [CD5]', name + b'S'), ] self.files = [ os.path.join(self.base, b'ABCD1234', b'cd 1', b'song1.mp3'), os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song2.mp3'), os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song3.mp3'), os.path.join(self.base, b'album', b'cd _ 1', b'song4.mp3'), - os.path.join(self.base, b'artist [CD5]', b'CÀT disc 1', b'song5.mp3'), - os.path.join(self.base, b'artist [CD5]', b'CÀt disc 2', b'song6.mp3'), - os.path.join(self.base, b'artist [CD5]', b'CÀTS', b'song7.mp3'), + os.path.join(self.base, b'artist [CD5]', name + b' disc 1', + b'song5.mp3'), + os.path.join(self.base, b'artist [CD5]', + name_alt_case + b' disc 2', b'song6.mp3'), + os.path.join(self.base, b'artist [CD5]', name + b'S', + b'song7.mp3'), ] for path in self.dirs: os.mkdir(path) - for path in self.files: - _mkmp3(path) + if files: + for path in self.files: + _mkmp3(path) def test_coalesce_nested_album_multiple_subdirs(self): + self.create_music() albums = list(albums_in_dir(self.base)) self.assertEquals(len(albums), 4) root, items = albums[0] @@ -1430,27 +1443,28 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): self.assertEquals(len(items), 3) def test_coalesce_nested_album_single_subdir(self): + self.create_music() albums = list(albums_in_dir(self.base)) root, items = albums[1] self.assertEquals(root, self.dirs[3:5]) self.assertEquals(len(items), 1) def test_coalesce_flattened_album_case_typo(self): + self.create_music() albums = list(albums_in_dir(self.base)) root, items = albums[2] self.assertEquals(root, self.dirs[6:8]) self.assertEquals(len(items), 2) def test_single_disc_album(self): + self.create_music() albums = list(albums_in_dir(self.base)) root, items = albums[3] self.assertEquals(root, self.dirs[8:]) self.assertEquals(len(items), 1) def test_do_not_yield_empty_album(self): - # Remove all the MP3s. - for path in self.files: - os.remove(path) + self.create_music(files=False) albums = list(albums_in_dir(self.base)) self.assertEquals(len(albums), 0) From 9de9d2497fcaf90aa5f8135411c1e1705302f840 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 31 Jan 2015 11:29:32 -0800 Subject: [PATCH 30/64] Unicode tests for #1285 --- test/test_importer.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_importer.py b/test/test_importer.py index 3a36fd65d..3a250988a 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1468,6 +1468,21 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): albums = list(albums_in_dir(self.base)) self.assertEquals(len(albums), 0) + def test_single_disc_unicode(self): + self.create_music(ascii=False) + albums = list(albums_in_dir(self.base)) + root, items = albums[3] + self.assertEquals(root, self.dirs[8:]) + self.assertEquals(len(items), 1) + + def test_coalesce_multiple_unicode(self): + self.create_music(ascii=False) + albums = list(albums_in_dir(self.base)) + self.assertEquals(len(albums), 4) + root, items = albums[0] + self.assertEquals(root, self.dirs[0:3]) + self.assertEquals(len(items), 3) + class ReimportTest(unittest.TestCase, ImportHelper): """Test "re-imports", in which the autotagging machinery is used for From 3f0dbb876d4d21989ea0ffe8ae39456b14c6b75c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 31 Jan 2015 11:54:32 -0800 Subject: [PATCH 31/64] Tests for #1285: normalize Unicode filenames --- test/test_importer.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index 3a250988a..45901bc6a 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # This file is part of beets. # Copyright 2015, Adrian Sampson. # @@ -22,6 +21,8 @@ import os import re import shutil import StringIO +import unicodedata +import sys from tempfile import mkstemp from zipfile import ZipFile from tarfile import TarFile @@ -1233,8 +1234,8 @@ class TagLogTest(_common.TestCase): sio = StringIO.StringIO() handler = logging.StreamHandler(sio) session = _common.import_session(loghandler=handler) - session.tag_log('status', u'café') # send unicode - self.assertIn(u'status café', sio.getvalue()) + session.tag_log('status', u'caf\xe9') # send unicode + self.assertIn(u'status caf\xe9', sio.getvalue()) class ResumeImportTest(unittest.TestCase, TestHelper): @@ -1390,8 +1391,8 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): self.base = os.path.abspath(os.path.join(self.temp_dir, b'tempdir')) os.mkdir(self.base) - name = b'CAT' if ascii else b'CÁT' - name_alt_case = b'CAt' if ascii else b'CÁt' + name = b'CAT' if ascii else u'C\xc1T'.encode('utf8') + name_alt_case = b'CAt' if ascii else u'C\xc1t'.encode('utf8') self.dirs = [ # Nested album, multiple subdirs. @@ -1417,8 +1418,10 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): ] self.files = [ os.path.join(self.base, b'ABCD1234', b'cd 1', b'song1.mp3'), - os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song2.mp3'), - os.path.join(self.base, b'ABCD1234', b'cd 3 - bonus', b'song3.mp3'), + os.path.join(self.base, b'ABCD1234', + b'cd 3 - bonus', b'song2.mp3'), + os.path.join(self.base, b'ABCD1234', + b'cd 3 - bonus', b'song3.mp3'), os.path.join(self.base, b'album', b'cd _ 1', b'song4.mp3'), os.path.join(self.base, b'artist [CD5]', name + b' disc 1', b'song5.mp3'), @@ -1428,12 +1431,25 @@ class MultiDiscAlbumsInDirTest(_common.TestCase): b'song7.mp3'), ] + if not ascii: + self.dirs = [self._normalize_path(p) for p in self.dirs] + self.files = [self._normalize_path(p) for p in self.files] + for path in self.dirs: os.mkdir(path) if files: for path in self.files: _mkmp3(path) + def _normalize_path(self, path): + """Normalize a path's Unicode combining form according to the + platform. + """ + path = path.decode('utf8') + norm_form = 'NFD' if sys.platform == 'darwin' else 'NFC' + path = unicodedata.normalize(norm_form, path) + return path.encode('utf8') + def test_coalesce_nested_album_multiple_subdirs(self): self.create_music() albums = list(albums_in_dir(self.base)) From 336bb9255ced0693d1c2b990caf831823a84f562 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 31 Jan 2015 13:04:11 -0800 Subject: [PATCH 32/64] chroma: Fix refactored `beet submit` Fix #1293. --- beetsplug/chroma.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/chroma.py b/beetsplug/chroma.py index f2af6d803..928f90479 100644 --- a/beetsplug/chroma.py +++ b/beetsplug/chroma.py @@ -240,7 +240,7 @@ def submit_items(log, userkey, items, chunksize=64): del data[:] for item in items: - fp = fingerprint_item(item) + fp = fingerprint_item(log, item) # Construct a submission dictionary for this item. item_data = { From 43e9044843645d9d91a082c327ce66c38700ff20 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Sat, 31 Jan 2015 22:11:43 +0100 Subject: [PATCH 33/64] Duplicate album-import summary shows old/new filesize E.g.: Old: 13 items, MP3, 256kbps, 44:32, 101MB New: 13 items, MP3, 320kbps, 44:31, 128MB Fix #1291 --- beets/library.py | 5 +++++ beets/ui/commands.py | 2 ++ 2 files changed, 7 insertions(+) diff --git a/beets/library.py b/beets/library.py index d521fc4a8..9b4d8ad27 100644 --- a/beets/library.py +++ b/beets/library.py @@ -595,6 +595,11 @@ class Item(LibModel): """ return int(os.path.getmtime(syspath(self.path))) + def filesize(self): + """Returns the size, in bytes, of the file. + """ + return os.path.getsize(syspath(self.path)) + # Model methods. def remove(self, delete=False, with_album=True): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 219f9cd73..b8f492a9e 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -439,8 +439,10 @@ def summarize_items(items, singleton): average_bitrate = sum([item.bitrate for item in items]) / len(items) total_duration = sum([item.length for item in items]) + total_filesize = sum([item.filesize() for item in items]) summary_parts.append('{0}kbps'.format(int(average_bitrate / 1000))) summary_parts.append(ui.human_seconds_short(total_duration)) + summary_parts.append(ui.human_bytes(total_filesize)) return ', '.join(summary_parts) From 51ab099145f4346c696c06c13f377ec972244b52 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 1 Feb 2015 15:35:24 +0100 Subject: [PATCH 34/64] Extend item.write() to embed images without changing item Fixes #1241 and geigerzaehler/beets-check#14 Before, `embed_item` would add the images to the item and then call `item.write()` to write the item data, including the image, to the file. This would trigger `item.store()` in the `after_write` hook of the check plugin. This would in turn try to persist the temporary `images` attribute of the item, resulting in an SQL error. --- beets/library.py | 20 ++++++++++++++------ beetsplug/embedart.py | 8 +++----- test/test_library.py | 43 ++++++++++++++++++++++++++++--------------- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/beets/library.py b/beets/library.py index 9b4d8ad27..d010811aa 100644 --- a/beets/library.py +++ b/beets/library.py @@ -500,12 +500,18 @@ class Item(LibModel): self.path = read_path - def write(self, path=None): + def write(self, path=None, tags=None): """Write the item's metadata to a media file. All fields in `_media_fields` are written to disk according to the values on this object. + `path` is the path of the mediafile to wirte the data to. It + defaults to the item's path. + + `tags` is a dictionary of additional metadata the should be + written to the file. + Can raise either a `ReadError` or a `WriteError`. """ if path is None: @@ -513,8 +519,10 @@ class Item(LibModel): else: path = normpath(path) - tags = dict(self) - plugins.send('write', item=self, path=path, tags=tags) + item_tags = dict(self) + if tags is not None: + item_tags.update(tags) + plugins.send('write', item=self, path=path, tags=item_tags) try: mediafile = MediaFile(syspath(path), @@ -522,7 +530,7 @@ class Item(LibModel): except (OSError, IOError, UnreadableFileError) as exc: raise ReadError(self.path, exc) - mediafile.update(tags) + mediafile.update(item_tags) try: mediafile.save() except (OSError, IOError, MutagenError) as exc: @@ -533,14 +541,14 @@ class Item(LibModel): self.mtime = self.current_mtime() plugins.send('after_write', item=self, path=path) - def try_write(self, path=None): + def try_write(self, path=None, tags=None): """Calls `write()` but catches and logs `FileOperationError` exceptions. Returns `False` an exception was caught and `True` otherwise. """ try: - self.write(path) + self.write(path, tags) return True except FileOperationError as exc: log.error("{0}", exc) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 6e3783030..b705ed7e7 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -148,13 +148,11 @@ class EmbedCoverArtPlugin(BeetsPlugin): try: self._log.debug(u'embedding {0}', displayable_path(imagepath)) - item['images'] = [self._mediafile_image(imagepath, maxwidth)] + image = self._mediafile_image(imagepath, maxwidth) except IOError as exc: self._log.warning(u'could not read image file: {0}', exc) - else: - # We don't want to store the image in the database. - item.try_write(itempath) - del item['images'] + return + item.try_write(path=itempath, tags={'images': [image]}) def embed_album(self, album, maxwidth=None, quiet=False): """Embed album art into all of the album's items. diff --git a/test/test_library.py b/test/test_library.py index f39227a65..3848f2b7c 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -35,6 +35,7 @@ from beets import util from beets import plugins from beets import config from beets.mediafile import MediaFile +from test.helper import TestHelper # Shortcut to path normalization. np = util.normpath @@ -1109,37 +1110,49 @@ class UnicodePathTest(_common.LibTestCase): self.i.write() -class WriteTest(_common.LibTestCase): +class WriteTest(unittest.TestCase, TestHelper): + def setUp(self): + self.setup_beets() + + def tearDown(self): + self.teardown_beets() + def test_write_nonexistant(self): - self.i.path = '/path/does/not/exist' - self.assertRaises(beets.library.ReadError, self.i.write) + item = self.create_item() + item.path = '/path/does/not/exist' + with self.assertRaises(beets.library.ReadError): + item.write() def test_no_write_permission(self): - path = os.path.join(self.temp_dir, 'file.mp3') - shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), path) + item = self.add_item_fixture() + path = item.path os.chmod(path, stat.S_IRUSR) try: - self.i.path = path - self.assertRaises(beets.library.WriteError, self.i.write) + self.assertRaises(beets.library.WriteError, item.write) finally: # Restore write permissions so the file can be cleaned up. os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) def test_write_with_custom_path(self): - custom_path = os.path.join(self.temp_dir, 'file.mp3') - self.i.path = os.path.join(self.temp_dir, 'item_file.mp3') - shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), custom_path) - shutil.copy(os.path.join(_common.RSRC, 'empty.mp3'), self.i.path) + item = self.add_item_fixture() + custom_path = os.path.join(self.temp_dir, 'custom.mp3') + shutil.copy(item.path, custom_path) - self.i['artist'] = 'new artist' + item['artist'] = 'new artist' self.assertNotEqual(MediaFile(custom_path).artist, 'new artist') - self.assertNotEqual(MediaFile(self.i.path).artist, 'new artist') + self.assertNotEqual(MediaFile(item.path).artist, 'new artist') - self.i.write(custom_path) + item.write(custom_path) self.assertEqual(MediaFile(custom_path).artist, 'new artist') - self.assertNotEqual(MediaFile(self.i.path).artist, 'new artist') + self.assertNotEqual(MediaFile(item.path).artist, 'new artist') + + def test_write_custom_tags(self): + item = self.add_item_fixture(artist='old artist') + item.write(tags={'artist': 'new artist'}) + self.assertNotEqual(item.artist, 'new artist') + self.assertEqual(MediaFile(item.path).artist, 'new artist') class ItemReadTest(unittest.TestCase): From e681449785ea4ee2a111c361743f35cee22252ef Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Sun, 1 Feb 2015 17:01:06 +0100 Subject: [PATCH 35/64] Added documentation FileFilterPlugin uses the new return value feature Some tweaks to get the code more readable --- beets/importer.py | 44 ++++++++++++++++------------------------- beetsplug/filefilter.py | 7 ++++--- docs/changelog.rst | 3 ++- docs/dev/plugins.rst | 8 +++++--- test/test_plugins.py | 20 +++++++++++++++++++ 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 9de68d2f1..c15b6b3f3 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -546,9 +546,11 @@ class ImportTask(object): return plugins.send('album_imported', lib=lib, album=self.album) - def emit_created(self, session): - """Send the `import_task_created` event for this task and return a list - of tasks which might be returned by plugins. + def handle_created(self, session): + """Send the `import_task_created` event for this task. Return a list of + tasks that should continue through the pipeline. By default, this is a + list containing only the task itself, but plugins can replace the task + with new ones. """ tasks = plugins.send('import_task_created', session=session, task=self) if not tasks: @@ -557,10 +559,7 @@ class ImportTask(object): # The plugins gave us a list of lists of task. Flatten it. flat_tasks = [] for inner in tasks: - if isinstance(inner, list): - flat_tasks += inner - else: - flat_tasks.append(inner) + flat_tasks += inner tasks = [t for t in flat_tasks if t] return tasks @@ -1020,16 +1019,14 @@ class ImportTaskFactory(object): if self.session.config['singletons']: for path in paths: tasks = self._create(self.singleton(path)) - if tasks: - for task in tasks: - yield task + for task in tasks: + yield task yield self.sentinel(dirs) else: tasks = self._create(self.album(paths, dirs)) - if tasks: - for task in tasks: - yield task + for task in tasks: + yield task # Produce the final sentinel for this toppath to indicate that # it is finished. This is usually just a SentinelImportTask, but @@ -1048,11 +1045,10 @@ class ImportTaskFactory(object): task. If `task` is None, do nothing. """ if task: - tasks = task.emit_created(self.session) - for task in tasks: - if not task.skip: - self.imported += 1 + tasks = task.handle_created(self.session) + self.imported += len(tasks) return tasks + return [] def paths(self): """Walk `self.toppath` and yield `(dirs, files)` pairs where @@ -1205,8 +1201,7 @@ def query_tasks(session): # Search for items. for item in session.lib.items(session.query): task = SingletonImportTask(None, item) - tasks = task.emit_created(session) - for task in tasks: + for task in task.handle_created(session): yield task else: @@ -1223,8 +1218,7 @@ def query_tasks(session): item.album_id = None task = ImportTask(None, [album.item_dir()], items) - tasks = task.emit_created(session) - for task in tasks: + for task in task.handle_created(session): yield task @@ -1272,8 +1266,7 @@ def user_query(session, task): def emitter(task): for item in task.items: task = SingletonImportTask(task.toppath, item) - tasks = task.emit_created(session) - for new_task in tasks: + for new_task in task.handle_created(session): yield new_task yield SentinelImportTask(task.toppath, task.paths) @@ -1384,9 +1377,6 @@ def manipulate_files(session, task): def log_files(session, task): """A coroutine (pipeline stage) to log each file to be imported. """ - if task.skip: - return - if isinstance(task, SingletonImportTask): log.info(u'Singleton: {0}', displayable_path(task.item['path'])) elif task.items: @@ -1413,7 +1403,7 @@ def group_albums(session): tasks = [] for _, items in itertools.groupby(task.items, group): task = ImportTask(items=list(items)) - tasks += task.emit_created(session) + tasks += task.handle_created(session) tasks.append(SentinelImportTask(task.toppath, task.paths)) task = pipeline.multiple(tasks) diff --git a/beetsplug/filefilter.py b/beetsplug/filefilter.py index 1a19d46cc..fbb23aae7 100644 --- a/beetsplug/filefilter.py +++ b/beetsplug/filefilter.py @@ -18,7 +18,7 @@ import re from beets import config from beets.plugins import BeetsPlugin -from beets.importer import action, SingletonImportTask +from beets.importer import SingletonImportTask class FileFilterPlugin(BeetsPlugin): @@ -50,10 +50,11 @@ class FileFilterPlugin(BeetsPlugin): if len(items_to_import) > 0: task.items = items_to_import else: - task.choice_flag = action.SKIP + return [] elif isinstance(task, SingletonImportTask): if not self.file_filter(task.item['path']): - task.choice_flag = action.SKIP + return [] + return [task] def file_filter(self, full_path): """Checks if the configured regular expressions allow the import diff --git a/docs/changelog.rst b/docs/changelog.rst index b8fc19dac..8494c4823 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -96,7 +96,8 @@ For developers: should!) use modern ``{}``-style string formatting lazily. See :ref:`plugin-logging` in the plugin API docs. * A new ``import_task_created`` event lets you manipulate import tasks - immediately after they are initialized. + immediately after they are initialized. It's also possible to replace the + originally created tasks by returning new ones using this event. 1.3.10 (January 5, 2015) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index e8f32d25f..97da193f0 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -175,9 +175,11 @@ The events currently available are: written to disk (i.e., just after the file on disk is closed). * *import_task_created*: called immediately after an import task is - initialized. Plugins can use this to, for example, cancel processing of a - task before anything else happens. ``task`` (an `ImportTask`) and - ``session`` (an `ImportSession`). + initialized. Plugins can use this to, for example, change imported files of a + task before anything else happens. It's also possible to replace the task + with another task by returning a list of tasks. This list can contain zero + or more `ImportTask`s. Returning an empty list will stop the task. + Parameters: ``task`` (an `ImportTask`) and ``session`` (an `ImportSession`). * *import_task_start*: called when before an import task begins processing. Parameters: ``task`` and ``session``. diff --git a/test/test_plugins.py b/test/test_plugins.py index 056be14e4..d46c5bd5d 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -207,6 +207,26 @@ class EventsTest(unittest.TestCase, ImportHelper, TestHelper): self.file_paths.append(dest_path) def test_import_task_created(self): + import_files = [self.import_dir] + self._setup_import_session(singletons=False) + self.importer.paths = import_files + + with helper.capture_log() as logs: + self.importer.run() + self.unload_plugins() + + # Exactly one event should have been imported (for the album). + # Sentinels do not get emitted. + self.assertEqual(logs.count('Sending event: import_task_created'), 1) + + logs = [line for line in logs if not line.startswith('Sending event:')] + self.assertEqual(logs, [ + 'Album: {0}'.format(os.path.join(self.import_dir, 'album')), + ' {0}'.format(self.file_paths[0]), + ' {0}'.format(self.file_paths[1]), + ]) + + def test_import_task_created_with_plugin(self): class ToSingletonPlugin(plugins.BeetsPlugin): def __init__(self): super(ToSingletonPlugin, self).__init__() From a2b00a8408b51f7620f9a91072344b0b57792e7b Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Sun, 1 Feb 2015 20:31:03 +0100 Subject: [PATCH 36/64] Changelog entry for #1291 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b8fc19dac..20f2c6a27 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,8 @@ Changelog Features: +* The summary shown to compare duplicate albums during import now displays + the old and new filesizes. :bug:`1291` * The colors used are now configurable via the new config option ``colors``, nested under the option ``ui``. The `color` config option has been moved from top-level to under ``ui``. Beets will respect the old color setting, From b68708f81e0f893ccd7ccc1a34de093f2ce1947f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sun, 1 Feb 2015 12:22:24 -0800 Subject: [PATCH 37/64] Installation instructions for Fedora (fix #375) --- docs/guides/main.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/guides/main.rst b/docs/guides/main.rst index 1f0434a17..7c5e52082 100644 --- a/docs/guides/main.rst +++ b/docs/guides/main.rst @@ -40,6 +40,16 @@ You will need Python. (Beets is written for `Python 2.7`_, but it works with * For **Slackware**, there's a `SlackBuild`_ available. +* On **Fedora 21**, you there is a `copr`_ for beets, which you can install + using `DNF`_ like so:: + + $ yum install dnf dnf-plugins-core + $ dnf copr enable afreof/beets + $ yum update + $ yum install beets + +.. _copr: https://copr.fedoraproject.org/coprs/afreof/beets/ +.. _dnf: http://fedoraproject.org/wiki/Features/DNF .. _SlackBuild: http://slackbuilds.org/repository/14.1/multimedia/beets/ .. _beets port: http://portsmon.freebsd.org/portoverview.py?category=audio&portname=beets .. _beets from AUR: http://aur.archlinux.org/packages.php?ID=39577 From b150d4039469e3cad970eb7d6af17d6f23f700f3 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 1 Feb 2015 21:45:55 +0100 Subject: [PATCH 38/64] Changelog for #1294 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 20f2c6a27..ac20bb4c5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -89,6 +89,8 @@ Fixes: * The error message when MusicBrainz is not reachable on the network is now much clearer. Thanks to Tom Jaspers. :bug:`1190` :bug:`1272` * Improve error messages when parsing query strings with shlex. :bug:`1290` +* :doc:`/plugins/embedart`: Fix a crash that occured when used together + with the *check* plugin. :bug:`1241` For developers: From 482008bf1d87a89f1f9373ee64676c2e54b3e32b Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sun, 1 Feb 2015 17:25:06 +0100 Subject: [PATCH 39/64] Info plugin can filter properties in output Resolves #1287 --- beetsplug/info.py | 47 ++++++++++++++++++++++++++++++++++++++++++- docs/changelog.rst | 2 ++ docs/plugins/info.rst | 13 +++++++++++- test/helper.py | 2 +- test/test_info.py | 36 +++++++++++++++++++++------------ 5 files changed, 84 insertions(+), 16 deletions(-) diff --git a/beetsplug/info.py b/beetsplug/info.py index 9dd78f250..7a5a47b84 100644 --- a/beetsplug/info.py +++ b/beetsplug/info.py @@ -19,6 +19,7 @@ from __future__ import (division, absolute_import, print_function, unicode_literals) import os +import re from beets.plugins import BeetsPlugin from beets import ui @@ -77,7 +78,7 @@ def update_summary(summary, tags): def print_data(data): - path = data.pop('path') + path = data.pop('path', None) formatted = {} for key, value in data.iteritems(): if isinstance(value, list): @@ -85,6 +86,9 @@ def print_data(data): if value is not None: formatted[key] = value + if len(formatted) == 0: + return + maxwidth = max(len(key) for key in formatted) lineformat = u'{{0:>{0}}}: {{1}}'.format(maxwidth) @@ -107,6 +111,9 @@ class InfoPlugin(BeetsPlugin): help='show library fields instead of tags') cmd.parser.add_option('-s', '--summarize', action='store_true', help='summarize the tags of all files') + cmd.parser.add_option('-i', '--include-keys', default=[], + action='append', dest='included_keys', + help='comma separated list of keys to show') return [cmd] def run(self, lib, opts, args): @@ -128,6 +135,11 @@ class InfoPlugin(BeetsPlugin): else: data_collector = tag_data + included_keys = [] + for keys in opts.included_keys: + included_keys.extend(keys.split(',')) + key_filter = make_key_filter(included_keys) + first = True summary = {} for data_emitter in data_collector(lib, ui.decargs(args)): @@ -137,6 +149,9 @@ class InfoPlugin(BeetsPlugin): self._log.error(u'cannot read file: {0}', ex) continue + path = data.get('path') + data = key_filter(data) + data['path'] = path # always show path if opts.summarize: update_summary(summary, data) else: @@ -147,3 +162,33 @@ class InfoPlugin(BeetsPlugin): if opts.summarize: print_data(summary) + + +def make_key_filter(include): + """Return a function that filters a dictionary. + + The returned filter takes a dictionary and returns another + dictionary that only includes the key-value pairs where the key + glob-matches one of the keys in `include`. + """ + if not include: + return identity + + matchers = [] + for key in include: + key = re.escape(key) + key = key.replace(r'\*', '.*') + matchers.append(re.compile(key + '$')) + + def filter(data): + filtered = dict() + for key, value in data.items(): + if any(map(lambda m: m.match(key), matchers)): + filtered[key] = value + return filtered + + return filter + + +def identity(val): + return val diff --git a/docs/changelog.rst b/docs/changelog.rst index b8fc19dac..2f540814e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,6 +35,8 @@ Features: ``art_filename`` configuration option. :bug:`1258` * :doc:`/plugins/fetchart`: There's a new Wikipedia image source that uses DBpedia to find albums. Thanks to Tom Jaspers. :bug:`1194` +* :doc:`/plugins/info`: New options ``-i`` to display only given + properties. :bug:`1287` Core changes: diff --git a/docs/plugins/info.rst b/docs/plugins/info.rst index 944f17c55..5bfde0b41 100644 --- a/docs/plugins/info.rst +++ b/docs/plugins/info.rst @@ -18,7 +18,18 @@ your library:: $ beet info beatles -Command-line options include: +If you just want to see specific properties you can use the +``--include-keys`` option to filter them. The argument is a +comma-separated list of simple glob patterns where ``*`` matches any +string. For example:: + + $ beet info -i 'title,mb*' beatles + +Will only show the ``title`` property and all properties starting with +``mb``. You can add the ``-i`` option multiple times to the command +line. + +Additional command-line options include: * ``--library`` or ``-l``: Show data from the library database instead of the files' tags. diff --git a/test/helper.py b/test/helper.py index 81a0b00cc..64db9e8b1 100644 --- a/test/helper.py +++ b/test/helper.py @@ -407,7 +407,7 @@ class TestHelper(object): def run_with_output(self, *args): with capture_stdout() as out: self.run_command(*args) - return out.getvalue() + return out.getvalue().decode('utf-8') # Safe file operations diff --git a/test/test_info.py b/test/test_info.py index 09668ceb2..d528517ed 100644 --- a/test/test_info.py +++ b/test/test_info.py @@ -19,6 +19,7 @@ from test._common import unittest from test.helper import TestHelper from beets.mediafile import MediaFile +from beets.util import displayable_path class InfoTest(unittest.TestCase, TestHelper): @@ -52,17 +53,17 @@ class InfoTest(unittest.TestCase, TestHelper): self.assertNotIn('composer:', out) def test_item_query(self): - items = self.add_item_fixtures(count=2) - items[0].album = 'xxxx' - items[0].write() - items[0].album = 'yyyy' - items[0].store() + item1, item2 = self.add_item_fixtures(count=2) + item1.album = 'xxxx' + item1.write() + item1.album = 'yyyy' + item1.store() out = self.run_with_output('album:yyyy') - self.assertIn(items[0].path, out) - self.assertIn(b'album: xxxx', out) + self.assertIn(displayable_path(item1.path), out) + self.assertIn(u'album: xxxx', out) - self.assertNotIn(items[1].path, out) + self.assertNotIn(displayable_path(item2.path), out) def test_item_library_query(self): item, = self.add_item_fixtures() @@ -70,8 +71,8 @@ class InfoTest(unittest.TestCase, TestHelper): item.store() out = self.run_with_output('--library', 'album:xxxx') - self.assertIn(item.path, out) - self.assertIn(b'album: xxxx', out) + self.assertIn(displayable_path(item.path), out) + self.assertIn(u'album: xxxx', out) def test_collect_item_and_path(self): path = self.create_mediafile_fixture() @@ -88,9 +89,18 @@ class InfoTest(unittest.TestCase, TestHelper): mediafile.save() out = self.run_with_output('--summarize', 'album:AAA', path) - self.assertIn('album: AAA', out) - self.assertIn('tracktotal: 5', out) - self.assertIn('title: [various]', out) + self.assertIn(u'album: AAA', out) + self.assertIn(u'tracktotal: 5', out) + self.assertIn(u'title: [various]', out) + + def test_include_pattern(self): + item = self.add_item(album='xxxx') + + out = self.run_with_output('--library', 'album:xxxx', + '--include-keys', '*lbu*') + self.assertIn(displayable_path(item.path), out) + self.assertNotIn(u'title:', out) + self.assertIn(u'album: xxxx', out) def suite(): From 465719a2087c2ff75441e06e1e30064f64e05e39 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 10:13:48 -0800 Subject: [PATCH 40/64] scrub: Catch IOErrors in command (fix #1297) --- beetsplug/scrub.py | 7 +++++-- docs/changelog.rst | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index 5c0d74e6e..066387f74 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -70,8 +70,11 @@ class ScrubPlugin(BeetsPlugin): # Get album art if we need to restore it. if opts.write: - mf = mediafile.MediaFile(item.path, - config['id3v23'].get(bool)) + try: + mf = mediafile.MediaFile(item.path, + config['id3v23'].get(bool)) + except IOError as exc: + self._log.error(u'scrubbing failed: {0}', exc) art = mf.art # Remove all tags. diff --git a/docs/changelog.rst b/docs/changelog.rst index c0cf49b12..77ef019ab 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -93,6 +93,8 @@ Fixes: * Improve error messages when parsing query strings with shlex. :bug:`1290` * :doc:`/plugins/embedart`: Fix a crash that occured when used together with the *check* plugin. :bug:`1241` +* :doc:`/plugins/scrub`: Log an error instead of stopping when the ``beet + scrub`` command cannot write a file. :bug:`1297` For developers: From 313c3807aaf011fa2ccfe2d04d9c12126a1e51c3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 10:42:31 -0800 Subject: [PATCH 41/64] scrub: Use `syspath` in `beet scrub` (#1297) --- beetsplug/scrub.py | 7 ++++--- docs/changelog.rst | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index 066387f74..f6a3bed27 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -71,10 +71,11 @@ class ScrubPlugin(BeetsPlugin): # Get album art if we need to restore it. if opts.write: try: - mf = mediafile.MediaFile(item.path, + mf = mediafile.MediaFile(util.syspath(item.path), config['id3v23'].get(bool)) except IOError as exc: - self._log.error(u'scrubbing failed: {0}', exc) + self._log.error(u'could not open file to scrub: {0}', + exc) art = mf.art # Remove all tags. @@ -86,7 +87,7 @@ class ScrubPlugin(BeetsPlugin): item.try_write() if art: self._log.info(u'restoring art') - mf = mediafile.MediaFile(item.path) + mf = mediafile.MediaFile(util.syspath(item.path)) mf.art = art mf.save() diff --git a/docs/changelog.rst b/docs/changelog.rst index 77ef019ab..ba5934c4c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -94,7 +94,8 @@ Fixes: * :doc:`/plugins/embedart`: Fix a crash that occured when used together with the *check* plugin. :bug:`1241` * :doc:`/plugins/scrub`: Log an error instead of stopping when the ``beet - scrub`` command cannot write a file. :bug:`1297` + scrub`` command cannot write a file. Also, avoid problems on Windows with + Unicode filenames. :bug:`1297` For developers: From 6ed0b2e0f3455e4414938d254fef3ed7e21fcfb6 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Mon, 2 Feb 2015 21:52:23 +0100 Subject: [PATCH 42/64] Expose an Item's filesize as a field - Update test-case for info to make sure the item's path is pointing to an actual file. See #1291 --- beets/library.py | 7 ++----- beets/ui/commands.py | 2 +- test/test_info.py | 4 +++- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/beets/library.py b/beets/library.py index d010811aa..fd95b6d05 100644 --- a/beets/library.py +++ b/beets/library.py @@ -420,6 +420,8 @@ class Item(LibModel): def _getters(cls): getters = plugins.item_field_getters() getters['singleton'] = lambda i: i.album_id is None + # Filesize is given in bytes + getters['filesize'] = lambda i: os.path.getsize(syspath(i.path)) return getters @classmethod @@ -603,11 +605,6 @@ class Item(LibModel): """ return int(os.path.getmtime(syspath(self.path))) - def filesize(self): - """Returns the size, in bytes, of the file. - """ - return os.path.getsize(syspath(self.path)) - # Model methods. def remove(self, delete=False, with_album=True): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index b8f492a9e..d800493e5 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -439,7 +439,7 @@ def summarize_items(items, singleton): average_bitrate = sum([item.bitrate for item in items]) / len(items) total_duration = sum([item.length for item in items]) - total_filesize = sum([item.filesize() for item in items]) + total_filesize = sum([item.filesize for item in items]) summary_parts.append('{0}kbps'.format(int(average_bitrate / 1000))) summary_parts.append(ui.human_seconds_short(total_duration)) summary_parts.append(ui.human_bytes(total_filesize)) diff --git a/test/test_info.py b/test/test_info.py index d528517ed..e382f4872 100644 --- a/test/test_info.py +++ b/test/test_info.py @@ -94,7 +94,9 @@ class InfoTest(unittest.TestCase, TestHelper): self.assertIn(u'title: [various]', out) def test_include_pattern(self): - item = self.add_item(album='xxxx') + item, = self.add_item_fixtures() + item.album = 'xxxx' + item.store() out = self.run_with_output('--library', 'album:xxxx', '--include-keys', '*lbu*') From 1b22f122a8a56fd82dabd649a7fed50110b360c8 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 15:28:44 -0800 Subject: [PATCH 43/64] Binary literals in unique_path (fix #1298) --- beets/util/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index f6396f9da..01a0257b0 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -480,7 +480,7 @@ def unique_path(path): return path base, ext = os.path.splitext(path) - match = re.search(r'\.(\d)+$', base) + match = re.search(br'\.(\d)+$', base) if match: num = int(match.group(1)) base = base[:match.start()] @@ -488,7 +488,7 @@ def unique_path(path): num = 0 while True: num += 1 - new_path = '%s.%i%s' % (base, num, ext) + new_path = b'%s.%i%s' % (base, num, ext) if not os.path.exists(new_path): return new_path From 8151a40f1fe157750dbb995318240b319e73e3d3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 22:27:14 -0800 Subject: [PATCH 44/64] discogs: Catch socket errors (#1299) See also: https://github.com/discogs/discogs_client/issues/44 --- beetsplug/discogs.py | 5 ++++- docs/changelog.rst | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 86af9da06..d63aef8ae 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -90,6 +90,9 @@ class DiscogsPlugin(BeetsPlugin): token, secret = auth_client.get_access_token(code) except DiscogsAPIError: raise beets.ui.UserError('Discogs authorization failed') + except (ConnectionError, socket.error) as e: + self._log.debug(u'connection error: {0}', e) + raise beets.ui.UserError('communication with Discogs failed') # Save the token for later use. self._log.debug('Discogs token {0}, secret {1}', token, secret) @@ -122,7 +125,7 @@ class DiscogsPlugin(BeetsPlugin): except DiscogsAPIError as e: self._log.debug(u'API Error: {0} (query: {1})', e, query) return [] - except ConnectionError as e: + except (ConnectionError, socket.error) as e: self._log.debug(u'HTTP Connection Error: {0}', e) return [] diff --git a/docs/changelog.rst b/docs/changelog.rst index ba5934c4c..9be8779ea 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -96,6 +96,8 @@ Fixes: * :doc:`/plugins/scrub`: Log an error instead of stopping when the ``beet scrub`` command cannot write a file. Also, avoid problems on Windows with Unicode filenames. :bug:`1297` +* :doc:`/plugins/discogs`: Handle and log more kinds of communication + errors. :bug:`1299` For developers: From 77833f6c0562893193cc61838ca89b37c6e41485 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 22:34:22 -0800 Subject: [PATCH 45/64] Oops :flushed: #1299 --- beetsplug/discogs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d63aef8ae..2ba2fdd6f 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -30,6 +30,7 @@ import beets import re import time import json +import socket # Silence spurious INFO log lines generated by urllib3. From 9dc123a6655fb8882a2abcc6c5e44ddfca47808f Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Feb 2015 22:48:19 -0800 Subject: [PATCH 46/64] Changelog for filesize field See #1291. Feature in commit 6ed0b2e. --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9be8779ea..4ef2e1522 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -39,6 +39,8 @@ Features: DBpedia to find albums. Thanks to Tom Jaspers. :bug:`1194` * :doc:`/plugins/info`: New options ``-i`` to display only given properties. :bug:`1287` +* A new ``filesize`` field on items indicates the number of bytes in the file. + :bug:`1291` Core changes: From 1341ad9b1d39deea94ebe6614d5c14bbcec0cd77 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Tue, 3 Feb 2015 10:23:52 +0100 Subject: [PATCH 47/64] Importer UI: Display number of missing/unmatched tracks Fix #1088 --- beets/ui/commands.py | 8 ++++++-- docs/changelog.rst | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index d800493e5..32201f58d 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -359,14 +359,18 @@ def show_change(cur_artist, cur_album, match): # Missing and unmatched tracks. if match.extra_tracks: - print_('Missing tracks:') + print_('Missing tracks ({0}/{1} - {2:.1%}):'.format( + len(match.extra_tracks), + len(match.info.tracks), + len(match.extra_tracks) / len(match.info.tracks) + )) for track_info in match.extra_tracks: line = ' ! %s (#%s)' % (track_info.title, format_index(track_info)) if track_info.length: line += ' (%s)' % ui.human_seconds_short(track_info.length) print_(ui.colorize('text_warning', line)) if match.extra_items: - print_('Unmatched tracks:') + print_('Unmatched tracks ({0}):'.format(len(match.extra_items))) for item in match.extra_items: line = ' ! %s (#%s)' % (item.title, format_index(item)) if item.length: diff --git a/docs/changelog.rst b/docs/changelog.rst index 4ef2e1522..8dbdec5e0 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,6 +41,7 @@ Features: properties. :bug:`1287` * A new ``filesize`` field on items indicates the number of bytes in the file. :bug:`1291` +* The number of missing/unmatched tracks is shown during import. :bug:`1088` Core changes: From 11b446e7df89a4ec99deb239055d350defb0916b Mon Sep 17 00:00:00 2001 From: Malte Ried Date: Tue, 3 Feb 2015 12:15:17 +0100 Subject: [PATCH 48/64] Simplified the flattening for the lists returned by plugins on the import_task_created event. --- beets/importer.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index c15b6b3f3..6e7ffcd2c 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -556,11 +556,8 @@ class ImportTask(object): if not tasks: tasks = [self] else: - # The plugins gave us a list of lists of task. Flatten it. - flat_tasks = [] - for inner in tasks: - flat_tasks += inner - tasks = [t for t in flat_tasks if t] + # The plugins gave us a list of lists of tasks. Flatten it. + tasks = [t for inner in tasks for t in inner] return tasks def lookup_candidates(self): From 50dbd18493827b1d535112bb87bfad5f8451da91 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Tue, 3 Feb 2015 13:31:54 +0100 Subject: [PATCH 49/64] Duplicates: fix error caused by formatting Function arguement `format` was shadowing the built-in format function Fix #1300 --- beetsplug/duplicates.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/duplicates.py b/beetsplug/duplicates.py index 8f2359dce..fb697922b 100644 --- a/beetsplug/duplicates.py +++ b/beetsplug/duplicates.py @@ -27,7 +27,7 @@ PLUGIN = 'duplicates' def _process_item(item, lib, copy=False, move=False, delete=False, - tag=False, format=''): + tag=False, fmt=''): """Process Item `item` in `lib`. """ if copy: @@ -45,7 +45,7 @@ def _process_item(item, lib, copy=False, move=False, delete=False, raise UserError('%s: can\'t parse k=v tag: %s' % (PLUGIN, tag)) setattr(k, v) item.store() - print_(format(item, format)) + print_(format(item, fmt)) def _checksum(item, prog, log): @@ -229,7 +229,7 @@ class DuplicatesPlugin(BeetsPlugin): move=move, delete=delete, tag=tag, - format=fmt.format(obj_count)) + fmt=fmt.format(obj_count)) self._command.func = _dup return [self._command] From a7878b0eba864cd404284cbf98d151d48dc2dc89 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Feb 2015 09:50:36 -0800 Subject: [PATCH 50/64] A couple of explanatory comments for #1292 --- beetsplug/filefilter.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/beetsplug/filefilter.py b/beetsplug/filefilter.py index fbb23aae7..432388c5a 100644 --- a/beetsplug/filefilter.py +++ b/beetsplug/filefilter.py @@ -50,10 +50,15 @@ class FileFilterPlugin(BeetsPlugin): if len(items_to_import) > 0: task.items = items_to_import else: + # Returning an empty list of tasks from the handler + # drops the task from the rest of the importer pipeline. return [] + elif isinstance(task, SingletonImportTask): if not self.file_filter(task.item['path']): return [] + + # If not filtered, return the original task unchanged. return [task] def file_filter(self, full_path): From cc01d87209269fda478196fdb7ee43ce6a573756 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Feb 2015 23:04:14 -0800 Subject: [PATCH 51/64] discogs: Catch *another* exception (fix #1305) Everything but requests is a travesty. --- beetsplug/discogs.py | 10 +++++++--- docs/changelog.rst | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 2ba2fdd6f..a3239963b 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -31,6 +31,7 @@ import re import time import json import socket +import httplib # Silence spurious INFO log lines generated by urllib3. @@ -39,6 +40,9 @@ urllib3_logger.setLevel(logging.CRITICAL) USER_AGENT = u'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) +# Exceptions that discogs_client should really handle but does not. +CONNECTION_ERRORS = ConnectionError, socket.error, httplib.HTTPException + class DiscogsPlugin(BeetsPlugin): @@ -91,7 +95,7 @@ class DiscogsPlugin(BeetsPlugin): token, secret = auth_client.get_access_token(code) except DiscogsAPIError: raise beets.ui.UserError('Discogs authorization failed') - except (ConnectionError, socket.error) as e: + except CONNECTION_ERRORS as e: self._log.debug(u'connection error: {0}', e) raise beets.ui.UserError('communication with Discogs failed') @@ -126,7 +130,7 @@ class DiscogsPlugin(BeetsPlugin): except DiscogsAPIError as e: self._log.debug(u'API Error: {0} (query: {1})', e, query) return [] - except (ConnectionError, socket.error) as e: + except CONNECTION_ERRORS as e: self._log.debug(u'HTTP Connection Error: {0}', e) return [] @@ -154,7 +158,7 @@ class DiscogsPlugin(BeetsPlugin): if e.message != '404 Not Found': self._log.debug(u'API Error: {0} (query: {1})', e, result._uri) return None - except ConnectionError as e: + except CONNECTION_ERRORS as e: self._log.debug(u'HTTP Connection Error: {0}', e) return None return self.get_album_info(result) diff --git a/docs/changelog.rst b/docs/changelog.rst index e8473d7c5..81493428c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -100,7 +100,7 @@ Fixes: scrub`` command cannot write a file. Also, avoid problems on Windows with Unicode filenames. :bug:`1297` * :doc:`/plugins/discogs`: Handle and log more kinds of communication - errors. :bug:`1299` + errors. :bug:`1299` :bug:`1305` For developers: From 024778544024c52b82cb4ecd09b2cebbb231924c Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Feb 2015 23:42:47 -0800 Subject: [PATCH 52/64] discogs: Catch JSON decode errors Fix #1305 *again*. --- beetsplug/discogs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index a3239963b..267041bcc 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -41,7 +41,8 @@ urllib3_logger.setLevel(logging.CRITICAL) USER_AGENT = u'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) # Exceptions that discogs_client should really handle but does not. -CONNECTION_ERRORS = ConnectionError, socket.error, httplib.HTTPException +CONNECTION_ERRORS = (ConnectionError, socket.error, httplib.HTTPException, + ValueError) # JSON decoding raises a ValueError. class DiscogsPlugin(BeetsPlugin): From 5d49b24ea17d821dd76168618b3de9a4c523e21c Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Wed, 4 Feb 2015 12:55:55 +0100 Subject: [PATCH 53/64] Automatic path query detection on album queries - Path detection happens regardless of the model class - PathQuery can now match flexattr (in fast-mode) Fix #1302 --- beets/library.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/beets/library.py b/beets/library.py index fd95b6d05..43cac66e8 100644 --- a/beets/library.py +++ b/beets/library.py @@ -59,7 +59,7 @@ class PathQuery(dbcore.FieldQuery): return (item.path == self.file_path) or \ item.path.startswith(self.dir_path) - def clause(self): + def col_clause(self): escape = lambda m: self.escape_char + m.group(0) dir_pattern = self.escape_re.sub(escape, self.dir_path) dir_pattern = buffer(dir_pattern + b'%') @@ -1045,26 +1045,24 @@ def parse_query_parts(parts, model_cls): # Special-case path-like queries, which are non-field queries # containing path separators (/). - if 'path' in model_cls._fields: - path_parts = [] - non_path_parts = [] - for s in parts: - if s.find(os.sep, 0, s.find(':')) != -1: - # Separator precedes colon. - path_parts.append(s) - else: - non_path_parts.append(s) - else: - path_parts = () - non_path_parts = parts + path_parts = [] + non_path_parts = [] + for s in parts: + if s.find(os.sep, 0, s.find(':')) != -1: + # Separator precedes colon. + path_parts.append(s) + else: + non_path_parts.append(s) query, sort = dbcore.parse_sorted_query( model_cls, non_path_parts, prefixes ) # Add path queries to aggregate query. - if path_parts: - query.subqueries += [PathQuery('path', s) for s in path_parts] + # Match field / flexattr depending on whether the model has the path field + fast_query = 'path' in model_cls._fields + query.subqueries += [PathQuery('path', s, fast_query) for s in path_parts] + return query, sort From f1ce37e20eca369d593dbd9aa0e0581b6a7ed122 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 4 Feb 2015 09:34:41 -0800 Subject: [PATCH 54/64] Update Python download link --- docs/guides/main.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/main.rst b/docs/guides/main.rst index 7c5e52082..c636dd390 100644 --- a/docs/guides/main.rst +++ b/docs/guides/main.rst @@ -12,7 +12,7 @@ Installing You will need Python. (Beets is written for `Python 2.7`_, but it works with 2.6 as well. Python 3.x is not yet supported.) -.. _Python 2.7: http://www.python.org/download/releases/2.7.2/ +.. _Python 2.7: http://www.python.org/download/ * **Mac OS X** v10.7 (Lion) and 10.8 (Mountain Lion) include Python 2.7 out of the box; Snow Leopard ships with Python 2.6. From d8ebc71f98f348780156c46a53ebbb8fb5ab0121 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Thu, 5 Feb 2015 09:47:30 +0100 Subject: [PATCH 55/64] Auto path detection: use clearer variable name Per sampsyo's suggestion See #1302 --- beets/library.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/library.py b/beets/library.py index 43cac66e8..e807d386a 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1060,8 +1060,8 @@ def parse_query_parts(parts, model_cls): # Add path queries to aggregate query. # Match field / flexattr depending on whether the model has the path field - fast_query = 'path' in model_cls._fields - query.subqueries += [PathQuery('path', s, fast_query) for s in path_parts] + fast_path_query = 'path' in model_cls._fields + query.subqueries += [PathQuery('path', s, fast_path_query) for s in path_parts] return query, sort From 80c96d98deeb6951b79cb9ed52a07083706e18ef Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Thu, 5 Feb 2015 10:54:42 +0100 Subject: [PATCH 56/64] Oops, flake8 formatting --- beets/library.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index e807d386a..e4fbd1d49 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1061,7 +1061,8 @@ def parse_query_parts(parts, model_cls): # Add path queries to aggregate query. # Match field / flexattr depending on whether the model has the path field fast_path_query = 'path' in model_cls._fields - query.subqueries += [PathQuery('path', s, fast_path_query) for s in path_parts] + query.subqueries += [PathQuery('path', s, fast_path_query) + for s in path_parts] return query, sort From f4d971d3a843482e72fd80c10e7ec7eb50fb7dca Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Fri, 6 Feb 2015 10:36:23 +0100 Subject: [PATCH 57/64] PathQueryTest now also tests Album queries --- test/test_query.py | 55 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/test/test_query.py b/test/test_query.py index c76ddf11b..55980d0e1 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -60,8 +60,12 @@ class AnyFieldQueryTest(_common.LibTestCase): class AssertsMixin(object): def assert_matched(self, results, titles): + # TODO: refactor to "assert_items_matched" for clarity self.assertEqual([i.title for i in results], titles) + def assert_albums_matched(self, results, albums): + self.assertEqual([a.album for a in results], albums) + # A test case class providing a library with some dummy data and some # assertions involving that data. @@ -343,76 +347,119 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): super(PathQueryTest, self).setUp() self.i.path = '/a/b/c.mp3' self.i.title = 'path item' + self.i.album = 'path album' self.i.store() + self.lib.add_album([self.i]) def test_path_exact_match(self): q = 'path:/a/b/c.mp3' results = self.lib.items(q) self.assert_matched(results, ['path item']) + results = self.lib.albums(q) + self.assert_albums_matched(results, []) + def test_parent_directory_no_slash(self): q = 'path:/a' results = self.lib.items(q) self.assert_matched(results, ['path item']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) + def test_parent_directory_with_slash(self): q = 'path:/a/' results = self.lib.items(q) self.assert_matched(results, ['path item']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) + def test_no_match(self): q = 'path:/xyzzy/' results = self.lib.items(q) self.assert_matched(results, []) + results = self.lib.albums(q) + self.assert_albums_matched(results, []) + def test_fragment_no_match(self): q = 'path:/b/' results = self.lib.items(q) self.assert_matched(results, []) + results = self.lib.albums(q) + self.assert_albums_matched(results, []) + def test_nonnorm_path(self): q = 'path:/x/../a/b' results = self.lib.items(q) self.assert_matched(results, ['path item']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) + def test_slashed_query_matches_path(self): q = '/a/b' results = self.lib.items(q) self.assert_matched(results, ['path item']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) + def test_non_slashed_does_not_match_path(self): q = 'c.mp3' results = self.lib.items(q) self.assert_matched(results, []) + results = self.lib.albums(q) + self.assert_albums_matched(results, []) + def test_slashes_in_explicit_field_does_not_match_path(self): q = 'title:/a/b' results = self.lib.items(q) self.assert_matched(results, []) - def test_path_regex(self): + def test_path_item_regex(self): q = 'path::\\.mp3$' results = self.lib.items(q) self.assert_matched(results, ['path item']) + def test_path_album_regex(self): + q = 'path::b' + results = self.lib.albums(q) + self.assert_albums_matched(results, ['path album']) + def test_escape_underscore(self): - self.add_item(path='/a/_/title.mp3', title='with underscore') + self.add_album(path='/a/_/title.mp3', title='with underscore', + album='album with underscore') q = 'path:/a/_' results = self.lib.items(q) self.assert_matched(results, ['with underscore']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['album with underscore']) + def test_escape_percent(self): - self.add_item(path='/a/%/title.mp3', title='with percent') + self.add_album(path='/a/%/title.mp3', title='with percent', + album='album with percent') q = 'path:/a/%' results = self.lib.items(q) self.assert_matched(results, ['with percent']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['album with percent']) + def test_escape_backslash(self): - self.add_item(path=r'/a/\x/title.mp3', title='with backslash') + self.add_album(path=r'/a/\x/title.mp3', title='with backslash', + album='album with backslash') q = r'path:/a/\\x' results = self.lib.items(q) self.assert_matched(results, ['with backslash']) + results = self.lib.albums(q) + self.assert_albums_matched(results, ['album with backslash']) + class IntQueryTest(unittest.TestCase, TestHelper): From 049aa2f297ba4a3ee6a870f6a8474caa5a8938ef Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Fri, 6 Feb 2015 10:38:41 +0100 Subject: [PATCH 58/64] Album path queries get constructed automatically Ensures that relative path queries also work for -a mode --- beets/library.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beets/library.py b/beets/library.py index e4fbd1d49..40051c354 100644 --- a/beets/library.py +++ b/beets/library.py @@ -794,6 +794,10 @@ class Album(LibModel): _search_fields = ('album', 'albumartist', 'genre') + _types = { + 'path': PathType(), + } + _sorts = { 'albumartist': SmartArtistSort, 'artist': SmartArtistSort, From 3ec44aab3ecf6803f09702f1080e015aa7cb0a00 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Fri, 6 Feb 2015 10:42:46 +0100 Subject: [PATCH 59/64] Update `_types` field instead of assigning it Assigning it would override our pre-defined types (such as `path` in Album._types) --- beets/ui/__init__.py | 4 ++-- test/helper.py | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index e09043cbb..02a7a9478 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -839,8 +839,8 @@ def _setup(options, lib=None): if lib is None: lib = _open_library(config) plugins.send("library_opened", lib=lib) - library.Item._types = plugins.types(library.Item) - library.Album._types = plugins.types(library.Album) + library.Item._types.update(plugins.types(library.Item)) + library.Album._types.update(plugins.types(library.Album)) return subcommands, plugins, lib diff --git a/test/helper.py b/test/helper.py index 64db9e8b1..e929eecff 100644 --- a/test/helper.py +++ b/test/helper.py @@ -199,8 +199,11 @@ class TestHelper(object): beets.config['plugins'] = plugins beets.plugins.load_plugins(plugins) beets.plugins.find_plugins() - Item._types = beets.plugins.types(Item) - Album._types = beets.plugins.types(Album) + # Take a backup of the original _types to restore when unloading + Item._original_types = dict(Item._types) + Album._original_types = dict(Album._types) + Item._types.update(beets.plugins.types(Item)) + Album._types.update(beets.plugins.types(Album)) def unload_plugins(self): """Unload all plugins and remove the from the configuration. @@ -209,8 +212,8 @@ class TestHelper(object): beets.config['plugins'] = [] beets.plugins._classes = set() beets.plugins._instances = {} - Item._types = {} - Album._types = {} + Item._types = Item._original_types + Album._types = Album._original_types def create_importer(self, item_count=1, album_count=1): """Create files to import and return corresponding session. From 9b9c033df6bdedee0b76c6ea72d17bb5adc501fe Mon Sep 17 00:00:00 2001 From: David Logie Date: Sat, 7 Feb 2015 15:12:08 +0000 Subject: [PATCH 60/64] importfeeds: Fix Unicode error when writing files. --- beetsplug/importfeeds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/importfeeds.py b/beetsplug/importfeeds.py index d1d21f839..b1e76d9a8 100644 --- a/beetsplug/importfeeds.py +++ b/beetsplug/importfeeds.py @@ -63,7 +63,7 @@ def _write_m3u(m3u_path, items_paths): mkdirall(m3u_path) with open(syspath(m3u_path), 'a') as f: for path in items_paths: - f.write(path + '\n') + f.write(path + b'\n') class ImportFeedsPlugin(BeetsPlugin): From e7f8a627e94d175716816363288a062ae628298a Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Sat, 7 Feb 2015 12:51:54 -0800 Subject: [PATCH 61/64] Style fixes for pep8 1.6 --- beets/__init__.py | 6 +++--- beetsplug/bpd/gstplayer.py | 2 +- beetsplug/lyrics.py | 5 +++-- setup.cfg | 3 ++- test/_common.py | 2 +- test/test_web.py | 2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/beets/__init__.py b/beets/__init__.py index 8072d793b..751c77757 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -14,12 +14,12 @@ from __future__ import absolute_import, unicode_literals -__version__ = '1.3.11' -__author__ = 'Adrian Sampson ' - import beets.library from beets.util import confit +__version__ = '1.3.11' +__author__ = 'Adrian Sampson ' + Library = beets.library.Library config = confit.LazyConfig('beets', __name__) diff --git a/beetsplug/bpd/gstplayer.py b/beetsplug/bpd/gstplayer.py index a2872f7c4..adac1f5a4 100644 --- a/beetsplug/bpd/gstplayer.py +++ b/beetsplug/bpd/gstplayer.py @@ -29,7 +29,7 @@ import urllib import pygst pygst.require('0.10') -import gst +import gst # noqa class GstPlayer(object): diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 5ead7096e..7da5326b9 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -414,8 +414,9 @@ class Google(Backend): def fetch(self, artist, title): query = u"%s %s" % (artist, title) - url = u'https://www.googleapis.com/customsearch/v1?key=%s&cx=%s&q=%s' % \ - (self.api_key, self.engine_id, urllib.quote(query.encode('utf8'))) + url = u'https://www.googleapis.com/customsearch/v1?key=%s&cx=%s&q=%s' \ + % (self.api_key, self.engine_id, + urllib.quote(query.encode('utf8'))) data = urllib.urlopen(url) data = json.load(data) diff --git a/setup.cfg b/setup.cfg index 1944c4adb..40eda74ec 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,4 +6,5 @@ eval-attr="!=slow" [flake8] # E241 missing whitespace after ',' (used to align visually) # E221 multiple spaces before operator (used to align visually) -ignore=E241,E221 +# E731 do not assign a lambda expression, use a def +ignore=E241,E221,E731 diff --git a/test/_common.py b/test/_common.py index 723915d01..05adb23df 100644 --- a/test/_common.py +++ b/test/_common.py @@ -30,7 +30,7 @@ except ImportError: import unittest # Mangle the search path to include the beets sources. -sys.path.insert(0, '..') +sys.path.insert(0, '..') # noqa import beets.library from beets import importer, logging from beets.ui import commands diff --git a/test/test_web.py b/test/test_web.py index 72d9e61d1..9a8cb55e8 100644 --- a/test/test_web.py +++ b/test/test_web.py @@ -8,7 +8,7 @@ from test import _common import json import beetsplug from beets.library import Item, Album -beetsplug.__path__ = ['./beetsplug', '../beetsplug'] +beetsplug.__path__ = ['./beetsplug', '../beetsplug'] # noqa from beetsplug import web From 0a81aae142396569b1b20d03e76d6dde3ce4e438 Mon Sep 17 00:00:00 2001 From: Tom Jaspers Date: Sun, 8 Feb 2015 16:34:28 +0100 Subject: [PATCH 62/64] PathQueryTest: rename assert_matched for items --- test/test_query.py | 91 +++++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/test/test_query.py b/test/test_query.py index 55980d0e1..55450d0ad 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -59,8 +59,7 @@ class AnyFieldQueryTest(_common.LibTestCase): class AssertsMixin(object): - def assert_matched(self, results, titles): - # TODO: refactor to "assert_items_matched" for clarity + def assert_items_matched(self, results, titles): self.assertEqual([i.title for i in results], titles) def assert_albums_matched(self, results, albums): @@ -93,8 +92,8 @@ class DummyDataTestCase(_common.TestCase, AssertsMixin): self.lib.add(item) self.lib.add_album(items[:2]) - def assert_matched_all(self, results): - self.assert_matched(results, [ + def assert_items_matched_all(self, results): + self.assert_items_matched(results, [ 'foo bar', 'baz qux', 'beets 4 eva', @@ -105,72 +104,72 @@ class GetTest(DummyDataTestCase): def test_get_empty(self): q = '' results = self.lib.items(q) - self.assert_matched_all(results) + self.assert_items_matched_all(results) def test_get_none(self): q = None results = self.lib.items(q) - self.assert_matched_all(results) + self.assert_items_matched_all(results) def test_get_one_keyed_term(self): q = 'title:qux' results = self.lib.items(q) - self.assert_matched(results, ['baz qux']) + self.assert_items_matched(results, ['baz qux']) def test_get_one_keyed_regexp(self): q = r'artist::t.+r' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_get_one_unkeyed_term(self): q = 'three' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_get_one_unkeyed_regexp(self): q = r':x$' results = self.lib.items(q) - self.assert_matched(results, ['baz qux']) + self.assert_items_matched(results, ['baz qux']) def test_get_no_matches(self): q = 'popebear' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) def test_invalid_key(self): q = 'pope:bear' results = self.lib.items(q) # Matches nothing since the flexattr is not present on the # objects. - self.assert_matched(results, []) + self.assert_items_matched(results, []) def test_term_case_insensitive(self): q = 'oNE' results = self.lib.items(q) - self.assert_matched(results, ['foo bar']) + self.assert_items_matched(results, ['foo bar']) def test_regexp_case_sensitive(self): q = r':oNE' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) q = r':one' results = self.lib.items(q) - self.assert_matched(results, ['foo bar']) + self.assert_items_matched(results, ['foo bar']) def test_term_case_insensitive_with_key(self): q = 'artist:thrEE' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_key_case_insensitive(self): q = 'ArTiST:three' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_unkeyed_term_matches_multiple_columns(self): q = 'baz' results = self.lib.items(q) - self.assert_matched(results, [ + self.assert_items_matched(results, [ 'foo bar', 'baz qux', ]) @@ -178,7 +177,7 @@ class GetTest(DummyDataTestCase): def test_unkeyed_regexp_matches_multiple_columns(self): q = r':z$' results = self.lib.items(q) - self.assert_matched(results, [ + self.assert_items_matched(results, [ 'foo bar', 'baz qux', ]) @@ -186,41 +185,41 @@ class GetTest(DummyDataTestCase): def test_keyed_term_matches_only_one_column(self): q = 'title:baz' results = self.lib.items(q) - self.assert_matched(results, ['baz qux']) + self.assert_items_matched(results, ['baz qux']) def test_keyed_regexp_matches_only_one_column(self): q = r'title::baz' results = self.lib.items(q) - self.assert_matched(results, [ + self.assert_items_matched(results, [ 'baz qux', ]) def test_multiple_terms_narrow_search(self): q = 'qux baz' results = self.lib.items(q) - self.assert_matched(results, [ + self.assert_items_matched(results, [ 'baz qux', ]) def test_multiple_regexps_narrow_search(self): q = r':baz :qux' results = self.lib.items(q) - self.assert_matched(results, ['baz qux']) + self.assert_items_matched(results, ['baz qux']) def test_mixed_terms_regexps_narrow_search(self): q = r':baz qux' results = self.lib.items(q) - self.assert_matched(results, ['baz qux']) + self.assert_items_matched(results, ['baz qux']) def test_single_year(self): q = 'year:2001' results = self.lib.items(q) - self.assert_matched(results, ['foo bar']) + self.assert_items_matched(results, ['foo bar']) def test_year_range(self): q = 'year:2000..2002' results = self.lib.items(q) - self.assert_matched(results, [ + self.assert_items_matched(results, [ 'foo bar', 'baz qux', ]) @@ -228,22 +227,22 @@ class GetTest(DummyDataTestCase): def test_singleton_true(self): q = 'singleton:true' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_singleton_false(self): q = 'singleton:false' results = self.lib.items(q) - self.assert_matched(results, ['foo bar', 'baz qux']) + self.assert_items_matched(results, ['foo bar', 'baz qux']) def test_compilation_true(self): q = 'comp:true' results = self.lib.items(q) - self.assert_matched(results, ['foo bar', 'baz qux']) + self.assert_items_matched(results, ['foo bar', 'baz qux']) def test_compilation_false(self): q = 'comp:false' results = self.lib.items(q) - self.assert_matched(results, ['beets 4 eva']) + self.assert_items_matched(results, ['beets 4 eva']) def test_unknown_field_name_no_results(self): q = 'xyzzy:nonsense' @@ -270,7 +269,7 @@ class GetTest(DummyDataTestCase): q = u'title:caf\xe9' results = self.lib.items(q) - self.assert_matched(results, [u'caf\xe9']) + self.assert_items_matched(results, [u'caf\xe9']) def test_numeric_search_positive(self): q = dbcore.query.NumericQuery('year', '2001') @@ -354,7 +353,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_path_exact_match(self): q = 'path:/a/b/c.mp3' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) results = self.lib.albums(q) self.assert_albums_matched(results, []) @@ -362,7 +361,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_parent_directory_no_slash(self): q = 'path:/a' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) @@ -370,7 +369,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_parent_directory_with_slash(self): q = 'path:/a/' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) @@ -378,7 +377,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_no_match(self): q = 'path:/xyzzy/' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) results = self.lib.albums(q) self.assert_albums_matched(results, []) @@ -386,7 +385,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_fragment_no_match(self): q = 'path:/b/' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) results = self.lib.albums(q) self.assert_albums_matched(results, []) @@ -394,7 +393,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_nonnorm_path(self): q = 'path:/x/../a/b' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) @@ -402,7 +401,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_slashed_query_matches_path(self): q = '/a/b' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) results = self.lib.albums(q) self.assert_albums_matched(results, ['path album']) @@ -410,7 +409,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_non_slashed_does_not_match_path(self): q = 'c.mp3' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) results = self.lib.albums(q) self.assert_albums_matched(results, []) @@ -418,12 +417,12 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): def test_slashes_in_explicit_field_does_not_match_path(self): q = 'title:/a/b' results = self.lib.items(q) - self.assert_matched(results, []) + self.assert_items_matched(results, []) def test_path_item_regex(self): q = 'path::\\.mp3$' results = self.lib.items(q) - self.assert_matched(results, ['path item']) + self.assert_items_matched(results, ['path item']) def test_path_album_regex(self): q = 'path::b' @@ -435,7 +434,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): album='album with underscore') q = 'path:/a/_' results = self.lib.items(q) - self.assert_matched(results, ['with underscore']) + self.assert_items_matched(results, ['with underscore']) results = self.lib.albums(q) self.assert_albums_matched(results, ['album with underscore']) @@ -445,7 +444,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): album='album with percent') q = 'path:/a/%' results = self.lib.items(q) - self.assert_matched(results, ['with percent']) + self.assert_items_matched(results, ['with percent']) results = self.lib.albums(q) self.assert_albums_matched(results, ['album with percent']) @@ -455,7 +454,7 @@ class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin): album='album with backslash') q = r'path:/a/\\x' results = self.lib.items(q) - self.assert_matched(results, ['with backslash']) + self.assert_items_matched(results, ['with backslash']) results = self.lib.albums(q) self.assert_albums_matched(results, ['album with backslash']) @@ -564,11 +563,11 @@ class DefaultSearchFieldsTest(DummyDataTestCase): def test_items_matches_title(self): items = self.lib.items('beets') - self.assert_matched(items, ['beets 4 eva']) + self.assert_items_matched(items, ['beets 4 eva']) def test_items_does_not_match_year(self): items = self.lib.items('2001') - self.assert_matched(items, []) + self.assert_items_matched(items, []) class NoneQueryTest(unittest.TestCase, TestHelper): From 54887e7655351b58bf581fbd20f8ccadce367110 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sun, 1 Feb 2015 09:37:36 +0100 Subject: [PATCH 63/64] Widen usage of InvalidQueryError Replace previous InvalidQueryError with InvalidQueryArgumentTypeError which extends the former as TypeError. However they lack context: the query that caused the error. Raise an InvalidQueryError when a shell-like expression cannot be parsed by shlex. Improve #1290. --- beets/dbcore/__init__.py | 1 + beets/dbcore/query.py | 15 +++++++++++---- beets/library.py | 2 +- test/test_library.py | 7 +++++++ test/test_query.py | 9 +++++---- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/beets/dbcore/__init__.py b/beets/dbcore/__init__.py index 100f546b5..093591882 100644 --- a/beets/dbcore/__init__.py +++ b/beets/dbcore/__init__.py @@ -23,5 +23,6 @@ from .types import Type from .queryparse import query_from_strings from .queryparse import sort_from_strings from .queryparse import parse_sorted_query +from .query import InvalidQueryError # flake8: noqa diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 2f90e0398..8d9c1929c 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -24,11 +24,17 @@ from datetime import datetime, timedelta class InvalidQueryError(ValueError): + def __init__(self, query, explanation): + message = "Invalid query '{0}': {1}".format(query, explanation) + super(InvalidQueryError, self).__init__(message) + + +class InvalidQueryArgumentTypeError(InvalidQueryError, TypeError): def __init__(self, what, expected, detail=None): message = "'{0}' is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) - super(InvalidQueryError, self).__init__(message) + super(InvalidQueryArgumentTypeError, self).__init__(None, message) class Query(object): @@ -160,8 +166,9 @@ class RegexpQuery(StringFieldQuery): self.pattern = re.compile(self.pattern) except re.error as exc: # Invalid regular expression. - raise InvalidQueryError(pattern, "a regular expression", - format(exc)) + raise InvalidQueryArgumentTypeError(pattern, + "a regular expression", + format(exc)) @classmethod def string_match(cls, pattern, value): @@ -228,7 +235,7 @@ class NumericQuery(FieldQuery): try: return float(s) except ValueError: - raise InvalidQueryError(s, "an int or a float") + raise InvalidQueryArgumentTypeError(s, "an int or a float") def __init__(self, field, pattern, fast=True): super(NumericQuery, self).__init__(field, pattern, fast) diff --git a/beets/library.py b/beets/library.py index 40051c354..a9885918b 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1085,7 +1085,7 @@ def parse_query_string(s, model_cls): try: parts = [p.decode('utf8') for p in shlex.split(s)] except ValueError as exc: - raise ValueError("Cannot parse {0!r} (error was: {1})".format(s, exc)) + raise dbcore.InvalidQueryError(s, exc) return parse_query_parts(parts, model_cls) diff --git a/test/test_library.py b/test/test_library.py index 3848f2b7c..d3bfe1373 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -31,6 +31,7 @@ from test._common import unittest from test._common import item import beets.library import beets.mediafile +import beets.dbcore from beets import util from beets import plugins from beets import config @@ -1171,6 +1172,12 @@ class ItemReadTest(unittest.TestCase): item.read('/thisfiledoesnotexist') +class ParseQueryTest(unittest.TestCase): + def test_parse_invalid_query_string(self): + with self.assertRaises(beets.dbcore.InvalidQueryError): + beets.library.parse_query_string('foo"', None) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_query.py b/test/test_query.py index 55450d0ad..077518c36 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -23,8 +23,8 @@ from test import helper import beets.library from beets import dbcore -from beets.dbcore import types -from beets.dbcore.query import NoneQuery, InvalidQueryError +from beets.dbcore import types, InvalidQueryError +from beets.dbcore.query import NoneQuery, InvalidQueryArgumentTypeError from beets.library import Library, Item @@ -282,14 +282,15 @@ class GetTest(DummyDataTestCase): self.assertFalse(results) def test_invalid_query(self): - with self.assertRaises(InvalidQueryError) as raised: + with self.assertRaises(InvalidQueryArgumentTypeError) as raised: dbcore.query.NumericQuery('year', '199a') self.assertIn('not an int', unicode(raised.exception)) - with self.assertRaises(InvalidQueryError) as raised: + with self.assertRaises(InvalidQueryArgumentTypeError) as raised: dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', unicode(raised.exception)) self.assertIn('unbalanced parenthesis', unicode(raised.exception)) + self.assertIsInstance(raised.exception, (InvalidQueryError, TypeError)) class MatchTest(_common.TestCase): From f443e0bfc5d1114b1dbc182ea223aab23d9f6a49 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 9 Feb 2015 15:44:49 +0100 Subject: [PATCH 64/64] InvalidQueryArgumentTypeError does not extend InvalidQueryError Places where InvalidQueryArgumentTypeError may be raised (i.e. all current ones) may not know the query therefore it cannot be an InvalidQueryError. The InvalidQueryArgumentTypeError is caught in beets.library.Library._fetch() and an InvalidQueryError is then raised. Improve #1290. --- beets/dbcore/query.py | 17 ++++++++++++++--- beets/library.py | 13 ++++++++----- test/test_query.py | 4 ++-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index 8d9c1929c..3727f6d7f 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -24,17 +24,28 @@ from datetime import datetime, timedelta class InvalidQueryError(ValueError): + """Represent any kind of invalid query + + The query should be a unicode string or a list, which will be space-joined. + """ def __init__(self, query, explanation): - message = "Invalid query '{0}': {1}".format(query, explanation) + if isinstance(query, list): + query = " ".join(query) + message = "'{0}': {1}".format(query, explanation) super(InvalidQueryError, self).__init__(message) -class InvalidQueryArgumentTypeError(InvalidQueryError, TypeError): +class InvalidQueryArgumentTypeError(TypeError): + """Represent a query argument that could not be converted as expected. + + It exists to be caught in upper stack levels so a meaningful (i.e. with the + query) InvalidQueryError can be raised. + """ def __init__(self, what, expected, detail=None): message = "'{0}' is not {1}".format(what, expected) if detail: message = "{0}: {1}".format(message, detail) - super(InvalidQueryArgumentTypeError, self).__init__(None, message) + super(InvalidQueryArgumentTypeError, self).__init__(message) class Query(object): diff --git a/beets/library.py b/beets/library.py index a9885918b..a57ed1642 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1155,11 +1155,14 @@ class Library(dbcore.Database): in the query string the `sort` argument is ignored. """ # Parse the query, if necessary. - parsed_sort = None - if isinstance(query, basestring): - query, parsed_sort = parse_query_string(query, model_cls) - elif isinstance(query, (list, tuple)): - query, parsed_sort = parse_query_parts(query, model_cls) + try: + parsed_sort = None + if isinstance(query, basestring): + query, parsed_sort = parse_query_string(query, model_cls) + elif isinstance(query, (list, tuple)): + query, parsed_sort = parse_query_parts(query, model_cls) + except dbcore.query.InvalidQueryArgumentTypeError as exc: + raise dbcore.InvalidQueryError(query, exc) # Any non-null sort specified by the parsed query overrides the # provided sort. diff --git a/test/test_query.py b/test/test_query.py index 077518c36..a32d8d60d 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -23,7 +23,7 @@ from test import helper import beets.library from beets import dbcore -from beets.dbcore import types, InvalidQueryError +from beets.dbcore import types from beets.dbcore.query import NoneQuery, InvalidQueryArgumentTypeError from beets.library import Library, Item @@ -290,7 +290,7 @@ class GetTest(DummyDataTestCase): dbcore.query.RegexpQuery('year', '199(') self.assertIn('not a regular expression', unicode(raised.exception)) self.assertIn('unbalanced parenthesis', unicode(raised.exception)) - self.assertIsInstance(raised.exception, (InvalidQueryError, TypeError)) + self.assertIsInstance(raised.exception, TypeError) class MatchTest(_common.TestCase):