Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bracket argument to aunique #2399

Merged
merged 6 commits into from
Jan 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions beets/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -1447,13 +1447,15 @@ def tmpl_time(s, fmt):
cur_fmt = beets.config['time_format'].as_str()
return time.strftime(fmt, time.strptime(s, cur_fmt))

def tmpl_aunique(self, keys=None, disam=None):
def tmpl_aunique(self, keys=None, disam=None, bracket=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a description of bracket to the docstring?

"""Generate a string that is guaranteed to be unique among all
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.
whitespace-separated lists of field names, while "bracket" is a
pair of characters to be used as brackets surrounding the
disambiguator or empty to have no brackets.
"""
# Fast paths: no album, no item or library, or memoized value.
if not self.item or not self.lib:
Expand All @@ -1467,9 +1469,20 @@ def tmpl_aunique(self, keys=None, disam=None):

keys = keys or 'albumartist album'
disam = disam or 'albumtype year label catalognum albumdisambig'
if bracket is None:
bracket = '[]'
keys = keys.split()
disam = disam.split()

# Assign a left and right bracket or leave blank if argument is empty.
if len(bracket) == 2:
bracket = list(bracket)
bracket_l = bracket[0]
bracket_r = bracket[1]
else:
bracket_l = u''
bracket_r = u''

album = self.lib.get_album(self.item)
if not album:
# Do nothing for singletons.
Expand Down Expand Up @@ -1502,13 +1515,19 @@ def tmpl_aunique(self, keys=None, disam=None):

else:
# No disambiguator distinguished all fields.
res = u' {0}'.format(album.id)
res = u' {1}{0}{2}'.format(album.id, bracket_l, bracket_r)
self.lib._memotable[memokey] = res
return res

# Flatten disambiguation value into a string.
disam_value = album.formatted(True).get(disambiguator)
res = u' [{0}]'.format(disam_value)

# Return empty string if disambiguator is empty.
if disam_value:
res = u' {1}{0}{2}'.format(disam_value, bracket_l, bracket_r)
else:
res = u''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… is there a cleaner way we can check for an empty disambiguation string? Perhaps, if this appears earlier, we can just use if disam_value: for the check.


self.lib._memotable[memokey] = res
return res

Expand Down
5 changes: 5 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ New features:
return the item info for the file at the given path, or 404.
* The :doc:`/plugins/web` also has a new config option, ``include_paths``,
which will cause paths to be included in item API responses if set to true.
* The ``%aunique`` template function for :ref:`aunique` now takes a third
argument that specifies which brackets to use around the disambiguator
value. The argument can be any two characters that represent the left and
right brackets. It defaults to `[]` and can also be blank to turn off
bracketing. :bug:`2397` :bug:`2399`

Fixes:

Expand Down
26 changes: 16 additions & 10 deletions docs/reference/pathformat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ These functions are built in to beets:
For example, "café" becomes "cafe". Uses the mapping provided by the
`unidecode module`_. See the :ref:`asciify-paths` configuration
option.
* ``%aunique{identifiers,disambiguators}``: Provides a unique string to
disambiguate similar albums in the database. See :ref:`aunique`, below.
* ``%aunique{identifiers,disambiguators,brackets}``: Provides a unique string
to disambiguate similar albums in the database. See :ref:`aunique`, below.
* ``%time{date_time,format}``: Return the date and time in any format accepted
by `strftime`_. For example, to get the year some music was added to your
library, use ``%time{$added,%Y}``.
Expand Down Expand Up @@ -112,21 +112,27 @@ will expand to "[2008]" for one album and "[2010]" for the other. The
function detects that you have two albums with the same artist and title but
that they have different release years.

For full flexibility, the ``%aunique`` function takes two arguments, each of
which are whitespace-separated lists of album field names: a set of
*identifiers* and a set of *disambiguators*. Any group of albums with identical
values for all the identifiers will be considered "duplicates". Then, the
function tries each disambiguator field, looking for one that distinguishes each
of the duplicate albums from each other. The first such field is used as the
result for ``%aunique``. If no field suffices, an arbitrary number is used to
distinguish the two albums.
For full flexibility, the ``%aunique`` function takes three arguments. The
first two are whitespace-separated lists of album field names: a set of
*identifiers* and a set of *disambiguators*. The third argument is a pair of
characters used to surround the disambiguator.

Any group of albums with identical values for all the identifiers will be
considered "duplicates". Then, the function tries each disambiguator field,
looking for one that distinguishes each of the duplicate albums from each
other. The first such field is used as the result for ``%aunique``. If no field
suffices, an arbitrary number is used to distinguish the two albums.

The default identifiers are ``albumartist album`` and the default disambiguators
are ``albumtype year label catalognum albumdisambig``. So you can get reasonable
disambiguation behavior if you just use ``%aunique{}`` with no parameters in
your path forms (as in the default path formats), but you can customize the
disambiguation if, for example, you include the year by default in path formats.

The default characters used as brackets are ``[]``. To change this, provide a
third argument to the ``%aunique`` function consisting of two characters: the left
and right brackets. Or, to turn off bracketing entirely, leave argument blank.

One caveat: When you import an album that is named identically to one already in
your library, the *first* album—the one already in your library— will not
consider itself a duplicate at import time. This means that ``%aunique{}`` will
Expand Down
22 changes: 20 additions & 2 deletions test/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,8 +713,8 @@ def test_use_fallback_numbers_when_identical(self):
album2.year = 2001
album2.store()

self._assert_dest(b'/base/foo 1/the title', self.i1)
self._assert_dest(b'/base/foo 2/the title', self.i2)
self._assert_dest(b'/base/foo [1]/the title', self.i1)
self._assert_dest(b'/base/foo [2]/the title', self.i2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like the right change to me!


def test_unique_falls_back_to_second_distinguishing_field(self):
self._setf(u'foo%aunique{albumartist album,month year}/$title')
Expand All @@ -730,6 +730,24 @@ def test_unique_sanitized(self):
self._setf(u'foo%aunique{albumartist album,albumtype}/$title')
self._assert_dest(b'/base/foo [foo_bar]/the title', self.i1)

def test_drop_empty_disambig_string(self):
album1 = self.lib.get_album(self.i1)
album1.albumdisambig = None
album2 = self.lib.get_album(self.i2)
album2.albumdisambig = u'foo'
album1.store()
album2.store()
self._setf(u'foo%aunique{albumartist album,albumdisambig}/$title')
self._assert_dest(b'/base/foo/the title', self.i1)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I add an empty field in the tests? My efforts yielded [0000] for year = None and [1] for everything else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm; I suppose I assumed you had run into this with some music in your library. Maybe you can reuse the data from there? You could, for example, try setting some arbitrary field foo to be empty on one album and to have a known value on another, and then set up the %aunique invocation to use that as the only (or first) disambiguator.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that is what I ran into with my library. I think adding the second album fixed it.


def test_change_brackets(self):
self._setf(u'foo%aunique{albumartist album,year,()}/$title')
self._assert_dest(b'/base/foo (2001)/the title', self.i1)

def test_remove_brackets(self):
self._setf(u'foo%aunique{albumartist album,year,}/$title')
self._assert_dest(b'/base/foo 2001/the title', self.i1)


class PluginDestinationTest(_common.TestCase):
def setUp(self):
Expand Down