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/beets/importer.py b/beets/importer.py index 973097f9e..9de68d2f1 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -1475,7 +1475,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 + br'^%s\d' % re.escape(match.group(1)), re.I ) else: start_collapsing = False @@ -1497,7 +1497,7 @@ def albums_in_dir(path): # Set the current pattern to match directories with the same # prefix as this one, followed by a digit. collapse_pat = re.compile( - r'^%s\d' % re.escape(match.group(1)), re.I + br'^%s\d' % re.escape(match.group(1)), re.I ) break diff --git a/beets/library.py b/beets/library.py index 15151e9fe..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): @@ -1069,7 +1074,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/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) 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 = { diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 60ae3f032..f7116df15 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -46,6 +46,35 @@ def contains_feat(title): return bool(re.search(plugins.feat_tokens(), title, flags=re.IGNORECASE)) +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: + 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 + # 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 lhs: + feat_part = lhs + + return feat_part + + class FtInTitlePlugin(plugins.BeetsPlugin): def __init__(self): super(FtInTitlePlugin, self).__init__() @@ -125,27 +154,11 @@ class FtInTitlePlugin(plugins.BeetsPlugin): _, featured = split_on_feat(artist) if featured and albumartist != artist and albumartist: self._log.info(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: - self._log.info('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 + # Attempt to find the featured artist. + feat_part = find_feat_part(artist, albumartist) # If we have a featuring artist, move it to the title. if feat_part: 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: diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index aa0f4c6a7..435230fb1 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -26,6 +26,62 @@ 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 + }, + { + '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'] + ) + 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')) diff --git a/test/test_importer.py b/test/test_importer.py index 48a489258..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): @@ -1380,49 +1381,77 @@ 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. - self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir')) + `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 u'C\xc1T'.encode('utf8') + name_alt_case = b'CAt' if ascii else u'C\xc1t'.encode('utf8') + 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]', 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, 'artist [CD5]', 'CATS'), + os.path.join(self.base, b'artist [CD5]', name + b'S'), ] 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]', 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'), ] + 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) - for path in self.files: - _mkmp3(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)) self.assertEquals(len(albums), 4) root, items = albums[0] @@ -1430,30 +1459,46 @@ 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) + 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 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):