Commits

Adrian Sampson  committed 2849d7d

%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.

  • Participants
  • Parent commits f6cd9ee

Comments (0)

Files changed (2)

File beets/library.py

 
     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.
         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)

File test/test_db.py

         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