diff --git a/beets/library.py b/beets/library.py index 0ba29cd68..fc51a6c6a 100644 --- a/beets/library.py +++ b/beets/library.py @@ -30,7 +30,7 @@ from beets import logging from beets.mediafile import MediaFile, MutagenError, UnreadableFileError from beets import plugins from beets import util -from beets.util import bytestring_path, syspath, normpath, samefile +from beets.util import bytestring_path, syspath, normpath, samefile, legalize_path from beets.util.functemplate import Template from beets import dbcore from beets.dbcore import types @@ -742,71 +742,6 @@ class Item(LibModel): # Templating. - def _legalize_partial(self, path, fragment, replacements): - """ Perform a partial legalization of the path (ie. a single - sanitization and truncation). Outputs unicode if fragment is set, - otherwise bytes. - """ - - # Truncate components and remove forbidden characters. - path = util.sanitize_path(path, replacements) - - # Encode for the filesystem. - if not fragment: - path = bytestring_path(path) - - # Preserve extension. - _, extension = os.path.splitext(self.path) - if fragment: - # Outputting Unicode. - extension = extension.decode('utf8', 'ignore') - path += 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) - path = util.truncate_path(path, maxlen) - - return path - - def _legalize_path(self, path, fragment): - """ Perform several iterations of _legalize_partial, to generate a - stable, optimal output path. This is necessary for cases where - truncation produces unclean paths (eg. trailing space). - """ - - # Create a list of path candidates - path_candidates = [b''] - - replacements = self._db.replacements - - # Perform an initial pass - path = self._legalize_partial(path, fragment, replacements) - while path != path_candidates[-1]: - # This will keep sanitizing the path until it's stable, or an - # infinite loop appears - while path not in path_candidates: - path_candidates.append(path) - # Convert back to Unicode with extension removed - print(util.displayable_path(path)) - path = os.path.splitext(util.displayable_path(path))[0] - - # Run next pass - path = self._legalize_partial(path, fragment, replacements) - - # If an infinite loop occurred, adjust replacements to avoid it - if path != path_candidates[-1]: - replacements = dict((k, u'_') for k, v in replacements) - - # If there's a rule to match a single underscore, set the - # target to a blank string. - if '_' in replacements: - replacements['_'] = '' - - return path - def destination(self, fragment=False, basedir=None, platform=None, path_formats=None): """Returns the path in the library directory designated for the @@ -855,7 +790,13 @@ class Item(LibModel): if beets.config['asciify_paths']: subpath = unidecode(subpath) - subpath = self._legalize_path(subpath, fragment) + 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.legalize_path(subpath, self._db.replacements, maxlen, + os.path.splitext(self.path)[1], fragment) if fragment: return subpath diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 68be740f6..a82bb2c89 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -545,6 +545,59 @@ def truncate_path(path, length=MAX_FILENAME_LENGTH): return os.path.join(*out) +def _legalize_stage(path, replacements, length, extension, fragment): + """ Perform a signle stage of legalization of the path (ie. a single + sanitization and truncation). Returns the path (unicode if fragment is set, + otherwise bytes), 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): + """ Perform up to three calls to _legalize_stage, to generate a stable + output path, taking user replacements into consideration if possible. This + additional complexity is necessary for cases where truncation produces + unclean paths (eg. trailing space). + """ + + if fragment: + # Outputting Unicode. + extension = extension.decode('utf8', 'ignore') + + first_stage_path =\ + _legalize_stage(path, replacements, length, extension, fragment)[0] + + # Convert back to Unicode with extension removed. + first_stage_path = os.path.splitext(displayable_path(first_stage_path))[0] + + # 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. + if retruncated: + second_stage_path = _legalize_stage( + first_stage_path, None, length, extension, fragment + )[0] + + return second_stage_path + + def str2bool(value): """Returns a boolean reflecting a human-entered string.""" return value.lower() in ('yes', '1', 'true', 't', 'y') diff --git a/test/test_library.py b/test/test_library.py index e66ad28f0..30b0d6fc4 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -452,7 +452,7 @@ class DestinationTest(_common.TestCase): self.assertEqual(self.i.destination(), np('base/one/_.mp3')) - 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 = [ @@ -465,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):