diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py index fff72d5f0..c04e734b8 100644 --- a/beets/dbcore/query.py +++ b/beets/dbcore/query.py @@ -766,7 +766,10 @@ class FixedFieldSort(FieldSort): def order_clause(self): order = "ASC" if self.ascending else "DESC" if self.case_insensitive: - field = 'LOWER({})'.format(self.field) + field = '(CASE ' \ + 'WHEN TYPEOF({0})="text" THEN LOWER({0}) ' \ + 'WHEN TYPEOF({0})="blob" THEN LOWER({0}) ' \ + 'ELSE {0} END)'.format(self.field) else: field = self.field return "{0} {1}".format(field, order) diff --git a/beets/library.py b/beets/library.py index 006630c08..9d11b166c 100644 --- a/beets/library.py +++ b/beets/library.py @@ -790,26 +790,21 @@ class Item(LibModel): if beets.config['asciify_paths']: subpath = unidecode(subpath) - # Truncate components and remove forbidden characters. - subpath = util.sanitize_path(subpath, self._db.replacements) - - # Encode for the filesystem. - if not fragment: - subpath = bytestring_path(subpath) - - # Preserve extension. - _, extension = os.path.splitext(self.path) - if fragment: - # Outputting Unicode. - extension = extension.decode('utf8', 'ignore') - subpath += extension.lower() - - # Truncate too-long components. maxlen = beets.config['max_filename_length'].get(int) if not maxlen: # When zero, try to determine from filesystem. maxlen = util.max_filename_length(self._db.directory) - subpath = util.truncate_path(subpath, maxlen) + + subpath, fellback = util.legalize_path( + subpath, self._db.replacements, maxlen, + os.path.splitext(self.path)[1], fragment + ) + if fellback: + # Print an error message if legalization fell back to + # default replacements because of the maximum length. + log.warning('Fell back to default replacements when naming ' + 'file {}. Configure replacements to avoid lengthening ' + 'the filename.', subpath) if fragment: return subpath diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 00006ce51..bbdbbcf96 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1107,11 +1107,12 @@ def remove_items(lib, query, album, delete): print_() if delete: fmt = u'$path - $title' - prompt = 'Really DELETE %i files (y/n)?' % len(items) + prompt = 'Really DELETE %i file%s (y/n)?' % \ + (len(items), 's' if len(items) > 1 else '') else: fmt = '' - prompt = 'Really remove %i items from the library (y/n)?' % \ - len(items) + prompt = 'Really remove %i item%s from the library (y/n)?' % \ + (len(items), 's' if len(items) > 1 else '') # Show all the items. for item in items: @@ -1354,7 +1355,8 @@ def move_items(lib, dest, query, copy, album, pretend): action = 'Copying' if copy else 'Moving' entity = 'album' if album else 'item' - log.info(u'{0} {1} {2}s.', action, len(objs), entity) + log.info(u'{0} {1} {2}{3}.', action, len(objs), entity, + 's' if len(objs) > 1 else '') if pretend: if album: show_path_changes([(item.path, item.destination(basedir=dest)) @@ -1480,11 +1482,13 @@ def config_func(lib, opts, args): def config_edit(): """Open a program to edit the user configuration. + An empty config file is created if no existing config file exists. """ path = config.user_config_path() - editor = os.environ.get('EDITOR') try: + if not os.path.isfile(path): + open(path, 'w+').close() util.interactive_open(path, editor) except OSError as exc: message = "Could not edit configuration: {0}".format(exc) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 68be740f6..30a1fcba9 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -545,6 +545,78 @@ def truncate_path(path, length=MAX_FILENAME_LENGTH): return os.path.join(*out) +def _legalize_stage(path, replacements, length, extension, fragment): + """Perform a single round of path legalization steps + (sanitation/replacement, encoding from Unicode to bytes, + extension-appending, and truncation). Return the path (Unicode if + `fragment` is set, `bytes` otherwise) and whether truncation was + required. + """ + # Perform an initial sanitization including user replacements. + path = sanitize_path(path, replacements) + + # Encode for the filesystem. + if not fragment: + path = bytestring_path(path) + + # Preserve extension. + path += extension.lower() + + # Truncate too-long components. + pre_truncate_path = path + path = truncate_path(path, length) + + return path, path != pre_truncate_path + + +def legalize_path(path, replacements, length, extension, fragment): + """Given a path-like Unicode string, produce a legal path. Return + the path and a flag indicating whether some replacements had to be + ignored (see below). + + The legalization process (see `_legalize_stage`) consists of + applying the sanitation rules in `replacements`, encoding the string + to bytes (unless `fragment` is set), truncating components to + `length`, appending the `extension`. + + This function performs up to three calls to `_legalize_stage` in + case truncation conflicts with replacements (as can happen when + truncation creates whitespace at the end of the string, for + example). The limited number of iterations iterations avoids the + possibility of an infinite loop of sanitation and truncation + operations, which could be caused by replacement rules that make the + string longer. The flag returned from this function indicates that + the path has to be truncated twice (indicating that replacements + made the string longer again after it was truncated); the + application should probably log some sort of warning. + """ + + if fragment: + # Outputting Unicode. + extension = extension.decode('utf8', 'ignore') + + first_stage_path, _ = _legalize_stage( + path, replacements, length, extension, fragment + ) + + # Convert back to Unicode with extension removed. + first_stage_path, _ = os.path.splitext(displayable_path(first_stage_path)) + + # Re-sanitize following truncation (including user replacements). + second_stage_path, retruncated = _legalize_stage( + first_stage_path, replacements, length, extension, fragment + ) + + # If the path was once again truncated, discard user replacements + # and run through one last legalization stage. + if retruncated: + second_stage_path, _ = _legalize_stage( + first_stage_path, None, length, extension, fragment + ) + + return second_stage_path, retruncated + + def str2bool(value): """Returns a boolean reflecting a human-entered string.""" return value.lower() in ('yes', '1', 'true', 't', 'y') diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e19101e3e..3c9c0042a 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -388,6 +388,12 @@ class GStreamerBackend(Backend): self._res = self.Gst.ElementFactory.make("audioresample", "res") self._rg = self.Gst.ElementFactory.make("rganalysis", "rg") + if self._src is None or self._decbin is None or self._conv is None \ + or self._res is None or self._rg is None: + raise FatalReplayGainError( + "Failed to load required GStreamer plugins" + ) + # We check which files need gain ourselves, so all files given # to rganalsys should have their gain computed, even if it # already exists. diff --git a/docs/changelog.rst b/docs/changelog.rst index b085c271c..a0e7d70c3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -63,6 +63,14 @@ Fixes: * :doc:`/plugins/fetchart`: When album art is already present, the message is now printed in the ``text_highlight_minor`` color (light gray). Thanks to :user:`Somasis`. :bug:`1512` +* Some messages in the console UI now use plural nouns correctly. Thanks to + :user:`JesseWeinstein`. :bug:`1521` +* Sorting numerical fields (such as track) now works again. :bug:`1511` +* :doc:`/plugins/replaygain`: Missing GStreamer plugins now cause a helpful + error message instead of a crash. :bug:`1518` +* Fix an edge case when producing sanitized filenames where the maximum path + length conflicted with the :ref:`replace` rules. Thanks to Ben Ockmore. + :bug:`496` :bug:`1361` 1.3.13 (April 24, 2015) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index a97a8baa9..b1cf2deca 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -361,11 +361,11 @@ method. Here's an example plugin that provides a meaningless new field "foo":: - class fooplugin(beetsplugin): + class FooPlugin(BeetsPlugin): def __init__(self): - field = mediafile.mediafield( - mediafile.mp3descstoragestyle(u'foo') - mediafile.storagestyle(u'foo') + field = mediafile.MediaField( + mediafile.MP3DescStorageStyle(u'foo'), + mediafile.StorageStyle(u'foo') ) self.add_media_field('foo', field) diff --git a/docs/plugins/lyrics.rst b/docs/plugins/lyrics.rst index e7ecd1b04..1d91a00eb 100644 --- a/docs/plugins/lyrics.rst +++ b/docs/plugins/lyrics.rst @@ -49,10 +49,10 @@ configuration file. The available options are: - **google_engine_ID**: The custom search engine to use. Default: The `beets custom search engine`_, which gathers an updated list of sources known to be scrapeable. -- **sources**: List of sources to search for lyrics. An asterisk `*` expands +- **sources**: List of sources to search for lyrics. An asterisk ``*`` expands to all available sources. Default: ``google lyricwiki lyrics.com musixmatch``, i.e., all sources. - *google* source will be automatically deactivated if no `google_engine_ID` is + *google* source will be automatically deactivated if no ``google_API_key`` is setup. Here's an example of ``config.yaml``:: diff --git a/docs/reference/config.rst b/docs/reference/config.rst index d737bba69..9000f070a 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -119,6 +119,11 @@ compatibility with Windows-influenced network filesystems like Samba). Trailing dots and trailing whitespace, which can cause problems on Windows clients, are also removed. +When replacements other than the defaults are used, it is possible that they +will increase the length of the path. In the scenario where this leads to a +conflict with the maximum filename length, the default replacements will be +used to resolve the conflict and beets will display a warning. + Note that paths might contain special characters such as typographical quotes (``“”``). With the configuration above, those will not be replaced as they don't match the typewriter quote (``"``). To also strip these diff --git a/docs/reference/query.rst b/docs/reference/query.rst index 29c32a222..e2091bb14 100644 --- a/docs/reference/query.rst +++ b/docs/reference/query.rst @@ -100,7 +100,7 @@ While ordinary keywords perform simple substring matches, beets also supports regular expression matching for more advanced queries. To run a regex query, use an additional ``:`` between the field name and the expression:: - $ beet list 'artist::Ann(a|ie)' + $ beet list "artist::Ann(a|ie)" That query finds songs by Anna Calvi and Annie but not Annuals. Similarly, this query prints the path to any file in my library that's missing a track title:: @@ -110,11 +110,16 @@ query prints the path to any file in my library that's missing a track title:: To search *all* fields using a regular expression, just prefix the expression with a single ``:``, like so:: - $ beet list :Ho[pm]eless + $ beet list ":Ho[pm]eless" Regular expressions are case-sensitive and build on `Python's built-in implementation`_. See Python's documentation for specifics on regex syntax. +Most command-line shells will try to interpret common characters in regular +expressions, such as ``()[]|``. To type those characters, you'll need to +escape them (e.g., with backslashes or quotation marks, depending on your +shell). + .. _Python's built-in implementation: http://docs.python.org/library/re.html diff --git a/test/test_library.py b/test/test_library.py index a4b4d08a8..30b0d6fc4 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -452,8 +452,7 @@ class DestinationTest(_common.TestCase): self.assertEqual(self.i.destination(), np('base/one/_.mp3')) - @unittest.skip('unimplemented: #496') - def test_truncation_does_not_conflict_with_replacement(self): + def test_legalize_path_one_for_one_replacement(self): # Use a replacement that should always replace the last X in any # path component with a Z. self.lib.replacements = [ @@ -466,7 +465,23 @@ class DestinationTest(_common.TestCase): # The final path should reflect the replacement. dest = self.i.destination() - self.assertTrue('XZ' in dest) + self.assertEqual(dest[-2:], 'XZ') + + def test_legalize_path_one_for_many_replacement(self): + # Use a replacement that should always replace the last X in any + # path component with four Zs. + self.lib.replacements = [ + (re.compile(r'X$'), u'ZZZZ'), + ] + + # Construct an item whose untruncated path ends with a Y but whose + # truncated version ends with an X. + self.i.title = 'X' * 300 + 'Y' + + # The final path should ignore the user replacement and create a path + # of the correct length, containing Xs. + dest = self.i.destination() + self.assertEqual(dest[-2:], 'XX') class ItemFormattedMappingTest(_common.LibTestCase): diff --git a/test/test_sort.py b/test/test_sort.py index 519d19c6e..e763b6167 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -34,21 +34,21 @@ class DummyDataTestCase(_common.TestCase): albums = [_common.album() for _ in range(3)] albums[0].album = "Album A" albums[0].genre = "Rock" - albums[0].year = "2001" + albums[0].year = 2001 albums[0].flex1 = "Flex1-1" albums[0].flex2 = "Flex2-A" albums[0].albumartist = "Foo" albums[0].albumartist_sort = None albums[1].album = "Album B" albums[1].genre = "Rock" - albums[1].year = "2001" + albums[1].year = 2001 albums[1].flex1 = "Flex1-2" albums[1].flex2 = "Flex2-A" albums[1].albumartist = "Bar" albums[1].albumartist_sort = None albums[2].album = "Album C" albums[2].genre = "Jazz" - albums[2].year = "2005" + albums[2].year = 2005 albums[2].flex1 = "Flex1-1" albums[2].flex2 = "Flex2-B" albums[2].albumartist = "Baz" @@ -67,6 +67,7 @@ class DummyDataTestCase(_common.TestCase): items[0].album_id = albums[0].id items[0].artist_sort = None items[0].path = "/path0.mp3" + items[0].track = 1 items[1].title = 'Baz qux' items[1].artist = 'Two' items[1].album = 'Baz' @@ -77,6 +78,7 @@ class DummyDataTestCase(_common.TestCase): items[1].album_id = albums[0].id items[1].artist_sort = None items[1].path = "/patH1.mp3" + items[1].track = 2 items[2].title = 'Beets 4 eva' items[2].artist = 'Three' items[2].album = 'Foo' @@ -87,6 +89,7 @@ class DummyDataTestCase(_common.TestCase): items[2].album_id = albums[1].id items[2].artist_sort = None items[2].path = "/paTH2.mp3" + items[2].track = 3 items[3].title = 'Beets 4 eva' items[3].artist = 'Three' items[3].album = 'Foo2' @@ -97,6 +100,7 @@ class DummyDataTestCase(_common.TestCase): items[3].album_id = albums[2].id items[3].artist_sort = None items[3].path = "/PATH3.mp3" + items[3].track = 4 for item in items: self.lib.add(item) @@ -399,6 +403,7 @@ class CaseSensitivityTest(DummyDataTestCase, _common.TestCase): item.flex2 = "flex2-A" item.album_id = album.id item.artist_sort = None + item.track = 10 self.lib.add(item) self.new_album = album @@ -451,6 +456,17 @@ class CaseSensitivityTest(DummyDataTestCase, _common.TestCase): self.assertEqual(results[0].flex1, 'Flex1-0') self.assertEqual(results[-1].flex1, 'flex1') + def test_case_sensitive_only_affects_text(self): + config['sort_case_insensitive'] = True + q = 'track+' + results = list(self.lib.items(q)) + # If the numerical values were sorted as strings, + # then ['1', '10', '2'] would be valid. + print([r.track for r in results]) + self.assertEqual(results[0].track, 1) + self.assertEqual(results[1].track, 2) + self.assertEqual(results[-1].track, 10) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)