From 4d4113e3a4a157c60e81b4075fec7310547880e3 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:15:09 -0500 Subject: [PATCH 01/13] 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/13] 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/13] 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/13] 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/13] 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 557330e994e6f645b42ffa2f31fe9f82ca3970b2 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Sat, 31 Jan 2015 14:16:15 +0100 Subject: [PATCH 06/13] 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 07/13] 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 08/13] 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 09/13] 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 10/13] 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 11/13] 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 12/13] 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 13/13] 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)