From 8f128876e2a946f833b2884f00e65ee8d2c39fba Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 24 Apr 2012 21:15:50 -0700 Subject: [PATCH] %aunique: use a single field instead of a sequence For a less cumbersome uniquifying string, only a single field value is now used instead of a prefix of a list of fields. The old semantics had two problems that made it both unnecessary and insufficient: - In the vast majority of cases, a single field suffices (year OR label OR catalog number, for example) and forcing the string to include many identical fields is unnecessary. - If the albums are very similar, a prefix may be insufficient; a better solution may be found with an arbitrary subset. (Of course, we can't afford to search the whole power set.) So we're going with a single field for now. This should cause far less confusion. --- beets/library.py | 37 ++++++++++++------------------------- test/test_db.py | 4 ++++ 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/beets/library.py b/beets/library.py index cfc9e2727..4d0fb0328 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1431,8 +1431,8 @@ class DefaultTemplateFunctions(object): def tmpl_aunique(self, keys=None, disam=None): """Generate a string that is guaranteed to be unique among all - albums in the library who share the same set of keys. Fields - from "disam" are used in the string if they are sufficient to + albums in the library who share the same set of keys. A fields + from "disam" is used in the string if one is sufficient to disambiguate the albums. Otherwise, a fallback opaque value is used. Both "keys" and "disam" should be given as whitespace-separated lists of field names. @@ -1459,35 +1459,22 @@ class DefaultTemplateFunctions(object): if len(albums) == 1: return u'' - # Find the minimum number of fields necessary to disambiguate - # the set of albums. - disambiguators = [] - for field in disam: - disambiguators.append(field) - - # Get the value tuple for each album for these - # disambiguators. - disam_values = set() - for a in albums: - values = [getattr(a, f) for f in disambiguators] - disam_values.add(tuple(values)) + # Find the first disambiguator that distinguishes the albums. + for disambiguator in disam: + # Get the value for each album for the current field. + disam_values = set([getattr(a, disambiguator) for a in albums]) - # If the set of unique tuples is equal to the number of + # If the set of unique values is equal to the number of # albums in the disambiguation set, we're done -- this is # sufficient disambiguation. if len(disam_values) == len(albums): break else: - # Even when using all of the disambiguating fields, we - # could not separate all the albums. Fall back to the unique - # album ID. + # No disambiguator distinguished all fields. return u' {}'.format(album.id) - # Flatten disambiguation values into a string. - values = [ - util.sanitize_for_path(unicode(getattr(album, f)), - self.pathmod, f) - for f in disambiguators - ] - return u' [{}]'.format(u' '.join(values)) + # Flatten disambiguation value into a string. + disam_value = util.sanitize_for_path(getattr(album, disambiguator), + self.pathmod, disambiguator) + return u' [{}]'.format(disam_value) diff --git a/test/test_db.py b/test/test_db.py index 6140998a3..b04ba2e15 100644 --- a/test/test_db.py +++ b/test/test_db.py @@ -516,6 +516,10 @@ class DisambiguationTest(unittest.TestCase, PathFormattingMixin): self._assert_dest('/base/foo 1/the title', self.i1) self._assert_dest('/base/foo 2/the title', self.i2) + def test_unique_falls_back_to_second_distinguishing_field(self): + self._setf(u'foo%aunique{albumartist album,month year}/$title') + self._assert_dest('/base/foo [2001]/the title', self.i1) + def test_unique_sanitized(self): album2 = self.lib.get_album(self.i2) album2.year = 2001